linux-kernel - Re: [linux-sunxi] [PATCH v3 2/2] media: cedrus: Add H264 decoding support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Feb 2019 18:46:38 +0100
From:   Jernej Škrabec <jernej.skrabec@...il.com>
To:     linux-sunxi@...glegroups.com, maxime.ripard@...tlin.com
Cc:     hans.verkuil@...co.com, acourbot@...omium.org,
        sakari.ailus@...ux.intel.com,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        tfiga@...omium.org, posciak@...omium.org,
        Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        Chen-Yu Tsai <wens@...e.org>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
        nicolas.dufresne@...labora.com, jenskuske@...il.com,
        jonas@...boo.se, ezequiel@...labora.com,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [linux-sunxi] [PATCH v3 2/2] media: cedrus: Add H264 decoding support

Dne torek, 12. februar 2019 ob 11:43:14 CET je Maxime Ripard napisal(a):
> Hi,
> 
> On Mon, Feb 11, 2019 at 08:21:31PM +0100, Jernej Škrabec wrote:
> > > +	reg = 0;
> > > +	/*
> > > +	 * FIXME: This bit tells the video engine to use the default
> > > +	 * quantization matrices. This will obviously need to be
> > > +	 * changed to support the profiles supporting custom
> > > +	 * quantization matrices.
> > > +	 */
> > > +	reg |= VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT;
> > 
> > This flag should not be needed anymore. From what I see, you correctly set
> > scaling matrix every time.
> 
> The scaling matrix control is optional, so I guess we should protect
> that by a check on whether that control has been set or not. What do
> you think?

Is it? Consider following snippet from your patch:

<snip>
{
	.id	= V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
	.elem_size	= sizeof(struct v4l2_ctrl_h264_scaling_matrix),
	.codec	= CEDRUS_CODEC_H264,
	.required	= true,
},
<snip>

Doesn't "required = true" mean that it's mandatory? But yes, if it's optional, 
then you should add a check.

Best regards,
Jernej