Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To:     linux-sunxi@googlegroups.com, maxime.ripard@bootlin.com
Cc:     hans.verkuil@cisco.com, acourbot@chromium.org,
        sakari.ailus@linux.intel.com,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        tfiga@chromium.org, posciak@chromium.org,
        Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
        Chen-Yu Tsai <wens@csie.org>, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org,
        nicolas.dufresne@collabora.com, jenskuske@gmail.com,
        jonas@kwiboo.se, ezequiel@collabora.com,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [linux-sunxi] [PATCH v3 2/2] media: cedrus: Add H264 decoding support
Date:   Tue, 12 Feb 2019 18:46:38 +0100
Message-ID: <3572175.7W1cnfb8sa@jernej-laptop>
In-Reply-To: <20190212104314.slytpbufwhf5ujv7@flea>
References: <cover.d3bb4d93da91ed5668025354ee1fca656e7d5b8b.1549895062.git-series.maxime.ripard@bootlin.com> <12916702.RFRCeC2GgE@jernej-laptop> <20190212104314.slytpbufwhf5ujv7@flea>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Dne torek, 12. februar 2019 ob 11:43:14 CET je Maxime Ripard napisal(a):
> Hi,
>=20
> On Mon, Feb 11, 2019 at 08:21:31PM +0100, Jernej =C5=A0krabec wrote:
> > > +	reg =3D 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 |=3D VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT;
> >=20
> > This flag should not be needed anymore. From what I see, you correctly =
set
> > scaling matrix every time.
>=20
> 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	=3D V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
	.elem_size	=3D sizeof(struct v4l2_ctrl_h264_scaling_matrix),
	.codec	=3D CEDRUS_CODEC_H264,
	.required	=3D true,
},
<snip>

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

Best regards,
Jernej