Re: [ANN] Report of Media Summit: Codecs

From: Tomasz Figa <tfiga@chromium.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Alexandre Courbot <acourbot@chromium.org>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Daniel Gomez <daniel@qtec.com>,
	Peter Griffin <peter.griffin@linaro.org>,
	Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Helen Koike <helen.koike@collabora.com>,
	Jan Schmidt <jan@centricular.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.org>,
	Michael Tretter <m.tretter@pengutronix.de>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Matthew Waters <matthew@centricular.com>
Subject: Re: [ANN] Report of Media Summit: Codecs
Date: Tue, 3 Dec 2019 16:08:00 +0900	[thread overview]
Message-ID: <CAAFQd5CQ_TArwzBJ-X+aWNQpYaEO07_5DY_miZGyvXKoyG_jsA@mail.gmail.com> (raw)
In-Reply-To: <b0ddcd59-cd54-67b8-af63-e7749403f868@xs4all.nl>

On Tue, Nov 19, 2019 at 11:18 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 11/2/19 1:50 PM, Hans Verkuil wrote:
>
> <snip>
>
> > Action Items
> > ------------
> >
> > Hans Verkuil:
> >
> > - Ask Cisco colleagues which bitrate-related parameters have to be per-frame for
> >   an encoder
> > - make stateful encoder infrastructure + documentation for the missing bits
> > - investigate using different sizes for metadata controls in the control framework:
> >   is this possible?
>
> The problem is with compound arrays. struct v4l2_ext_control just provides a size field
> for the total size and so for an array there is no way to discover the actual element
> size.
>
> That said, to my knowledge there are currently no compound arrays defined in the mainline
> kernel.

V4L2_CTRL_TYPE_H264_SLICE_PARAMS is a compound array, with one element
of the array for each slice in the bitstream buffer.

> So one option is to take the last reserved __u32 field and split it in a
> __u16 elem_size and a __u16 reserved2 field, or just use the full __u32 for the elem size
> and drop the reserved2 field.

__u16 would limit the element size to 64k. While I don't see any
controls bigger than that on the horizon (and with the current design
handling such big controls would add a lot of memcpy overhead anyway),
I wonder if that's a good assumption. I'd probably just go with __u32.

>
> Alternatively, we prohibit compound arrays for now and postpone making any changes to
> struct v4l2_ext_control until we actually need this.
>
> As long as it is not an array, then we can safely extend these compound control structs
> later. It requires some work in the control framework, but it isn't too bad.
>
> I'm in favor of implementing this, and for now prohibiting the use of compound arrays.
>
> It is really helpful making these codec state controls at least somewhat future-proof.
>
> Comments?
>
>         Hans
>
> >
> > Michael Tretter:
> >
> > - Support the new encoder stateful controls in the driver
> >
> > Tomasz Figa:
> >
> > - look up AMD encoder support
> >
> > Boris Brezillon:
> >
> > - send v3 of hantro g1 fixes
> >
> > Nicolas Dufresne:
> >
> > - look into multiview and sublayers support
> >
> > Paul Kocialkowski:
> >
> > - check metadata controls against the standards and update the docs if needed
> >
> > Ezequiel Garcia and Boris Brezillon:
> >
> > - add VP9 support
> >
>