[RFC,0/7] media: Clean and make H264 stateless uAPI public
mbox series

Message ID 20200623182809.1375-1-ezequiel@collabora.com
Headers show
Series
  • media: Clean and make H264 stateless uAPI public
Related show

Message

Ezequiel Garcia June 23, 2020, 6:28 p.m. UTC
The recent patch posted by Jernej (which I'm including for context),
encouraged me to address all the known issues in the uAPI.

I hope we can finally make this uAPI interface
public; it would be nice to address the other codec
interfaces so we can move the codec drivers out of staging.

It should be noted that there is already GStreamer native
support for this interface, which will be part of 1.18,
once it's released [1], as well as support in Chromium [2].

The basic idea here is to sanitize the interface,
making sure the structs are aligned to 64-bit,
adding reserved fields for padding where suitable.

These reserved fields can then be used to support future extensions,
in case such need appears.

In addition to this, moving the slice invariant fields
to the per-frame control, makes the frame-mode driver
implementation much nicer and the interface; see patch 6/7 for details.

I'm not adding a MAINTAINERS entry, but I'd like to do so,
so we make sure any uAPI changes are sent to those involved.

Another potential change is the addition of a "Since:" tag to the
control specification, so we can document which kernel version
added the interface. This might prove useful if reserved
fields are then used to extend the interface.

Finally, I'm sneaking here a change from Philipp Zabel
which apparently fell thru the cracks.

[1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/tree/master/sys/v4l2codecs
[2] https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/media/gpu/v4l2/

Ezequiel Garcia (5):
  fixup! media: uapi: h264: update reference lists
  media: uapi: h264: increase size of fields
  media: uapi: h264: pad v4l2_ctrl_h264_pps to 64-bit
  media: uapi: h264: Clean slice invariants syntax elements
  media: uapi: make H264 stateless codec interface public

Jernej Skrabec (1):
  media: uapi: h264: update reference lists

Philipp Zabel (1):
  media: uapi: h264: clarify pic_order_cnt_bit_size field

 .../media/v4l/ext-ctrls-codec.rst             | 135 ++++++++++++------
 drivers/media/v4l2-core/v4l2-ctrls.c          |  31 ++++
 drivers/media/v4l2-core/v4l2-h264.c           |   8 +-
 .../staging/media/hantro/hantro_g1_h264_dec.c |  21 ++-
 drivers/staging/media/hantro/hantro_h264.c    |   3 +-
 drivers/staging/media/hantro/hantro_hw.h      |   5 +-
 drivers/staging/media/rkvdec/rkvdec-h264.c    |   6 +-
 .../staging/media/sunxi/cedrus/cedrus_h264.c  |  15 +-
 include/media/v4l2-ctrls.h                    |   3 +-
 include/media/v4l2-h264.h                     |   5 +-
 .../linux/v4l2-h264-ctrls.h}                  |  73 ++++++----
 11 files changed, 194 insertions(+), 111 deletions(-)
 rename include/{media/h264-ctrls.h => uapi/linux/v4l2-h264-ctrls.h} (88%)

Comments

Nicolas Dufresne June 25, 2020, 2:42 p.m. UTC | #1
Le mardi 23 juin 2020 à 15:28 -0300, Ezequiel Garcia a écrit :
> The recent patch posted by Jernej (which I'm including for context),
> encouraged me to address all the known issues in the uAPI.
> 
> I hope we can finally make this uAPI interface
> public; it would be nice to address the other codec
> interfaces so we can move the codec drivers out of staging.
> 
> It should be noted that there is already GStreamer native
> support for this interface, which will be part of 1.18,
> once it's released [1], as well as support in Chromium [2].

Keep in mind, no one did a MultiView and/or SVC implementation in
userspace yet. Multiview seems to share a lot of interlacing, which
ffmpeg implementation covers, but some validation would be nice.

As for SVC, I only started looking at that this means for decoding. It
looks like buffer "holding" mechanism is mostly what we need, I'd would
simply like to verify, as I was told that scaling down might be
required when SVC stream is missing (but perhaps this is optional ?)

> 
> The basic idea here is to sanitize the interface,
> making sure the structs are aligned to 64-bit,
> adding reserved fields for padding where suitable.
> 
> These reserved fields can then be used to support future extensions,
> in case such need appears.
> 
> In addition to this, moving the slice invariant fields
> to the per-frame control, makes the frame-mode driver
> implementation much nicer and the interface; see patch 6/7 for details.
> 
> I'm not adding a MAINTAINERS entry, but I'd like to do so,
> so we make sure any uAPI changes are sent to those involved.
> 
> Another potential change is the addition of a "Since:" tag to the
> control specification, so we can document which kernel version
> added the interface. This might prove useful if reserved
> fields are then used to extend the interface.
> 
> Finally, I'm sneaking here a change from Philipp Zabel
> which apparently fell thru the cracks.

Thanks a lot of working on this.

> 
> [1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/tree/master/sys/v4l2codecs
> [2] https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/media/gpu/v4l2/
> 
> Ezequiel Garcia (5):
>   fixup! media: uapi: h264: update reference lists
>   media: uapi: h264: increase size of fields
>   media: uapi: h264: pad v4l2_ctrl_h264_pps to 64-bit
>   media: uapi: h264: Clean slice invariants syntax elements
>   media: uapi: make H264 stateless codec interface public
> 
> Jernej Skrabec (1):
>   media: uapi: h264: update reference lists
> 
> Philipp Zabel (1):
>   media: uapi: h264: clarify pic_order_cnt_bit_size field
> 
>  .../media/v4l/ext-ctrls-codec.rst             | 135 ++++++++++++------
>  drivers/media/v4l2-core/v4l2-ctrls.c          |  31 ++++
>  drivers/media/v4l2-core/v4l2-h264.c           |   8 +-
>  .../staging/media/hantro/hantro_g1_h264_dec.c |  21 ++-
>  drivers/staging/media/hantro/hantro_h264.c    |   3 +-
>  drivers/staging/media/hantro/hantro_hw.h      |   5 +-
>  drivers/staging/media/rkvdec/rkvdec-h264.c    |   6 +-
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  |  15 +-
>  include/media/v4l2-ctrls.h                    |   3 +-
>  include/media/v4l2-h264.h                     |   5 +-
>  .../linux/v4l2-h264-ctrls.h}                  |  73 ++++++----
>  11 files changed, 194 insertions(+), 111 deletions(-)
>  rename include/{media/h264-ctrls.h => uapi/linux/v4l2-h264-ctrls.h} (88%)
>