On Wed, 26 Jun 2019 13:30:38 +0200 Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> wrote: > Hi, > > On Mon, 2019-06-10 at 10:52 +0200, Boris Brezillon wrote: > > Some stateless decoders don't support per-slice decoding (or at least > > not in a way that would make them efficient or easy to use). > > Let's expose a menu to control and expose the supported decoding modes. > > Drivers are allowed to support only one decoding but they can support > > both too. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> > > --- > > Changes in v2: > > * Allow decoding multiple slices in per-slice decoding mode > > * Minor doc improvement/fixes > > --- > > .../media/uapi/v4l/ext-ctrls-codec.rst | 46 ++++++++++++++++++- > > drivers/media/v4l2-core/v4l2-ctrls.c | 9 ++++ > > include/media/h264-ctrls.h | 13 ++++++ > > 3 files changed, 67 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > index 82547d5de250..e3b9ab73a588 100644 > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > @@ -1748,6 +1748,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > * - __u32 > > - ``size`` > > - > > + * - __u32 > > + - ``start_byte_offset`` > > + - Where the slice payload starts in the output buffer. Useful when > > + operating in per frame decoding mode and decoding multi-slice content. > > + In this case, the output buffer will contain more than one slice and > > + some codecs need to know where each slice starts. Note that this > > + offsets points to the beginning of the slice which is supposed to > > + contain an ANNEX B start code. > > * - __u32 > > - ``header_bit_size`` > > - > > @@ -1931,7 +1939,13 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > - > > * - __u16 > > - ``num_slices`` > > - - Number of slices needed to decode the current frame > > + - Number of slices attached to decode the operation. When operating in > > + per-frame decoding mode (see > > + :c:type:`v4l2_mpeg_video_h264_decoding_mode`), this field should > > + be set to the number of slices needed to fully decode the frame. When > > + operating in per-slice decoding mode this field can be set to anything > > + between 1 and the remaining number of slices needed to fully decode the > > + frame. > > * - __u16 > > - ``nal_ref_idc`` > > - NAL reference ID value coming from the NAL Unit header > > @@ -2022,6 +2036,36 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > - 0x00000004 > > - The DPB entry is a long term reference frame > > > > +``V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE (enum)`` > > + Specifies the decoding mode to use. Currently exposes per slice and per > > + frame decoding but new modes might be added later on. > > I would maybe formulate this as slice-based and frame-based decoding > since I feel like per-slice implies that slices have to be passed one- > by-one. This wouldn't be the case with the latest proposal for slice- > based decoding, where more than one slice can be passed at a time. > > About that, I'm wondering how we could handle that in our drivers. > It will probably be something like: > > device_run -+-> decode slice i -> IRQ -+-> job_finish > `----------- i++ ----------' > > And I'm wondering if there could be common helpers to help implement > this, if that's needed at all. Yep, we could probably have that kind of helper. I haven't had time to work on the generic m2m stateless codec layer since last time we talked about it on IRC, but I plan to resume working on this task soon. I'll try to think about this generic "decode all slices" helper once the basic building blocks are ready. > > What do you think? > > Anyway if you agree with the rewording, this is: I'm perfectly fine with the suggested rewording. > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> Thanks for the review. Boris
