[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3572175.7W1cnfb8sa@jernej-laptop>
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
Powered by blists - more mailing lists