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

[Jernej Åkrabec]

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