Re: [PATCH v4 04/11] media: uapi: h264: Add the concept of start code

From: Hans Verkuil
Date: Mon Aug 12 2019 - 07:57:58 EST


On 8/12/19 1:56 PM, Ezequiel Garcia wrote:
> On Mon, 2019-08-12 at 12:11 +0200, Hans Verkuil wrote:
>> On 8/8/19 12:34 PM, Ezequiel Garcia wrote:
>>> Stateless decoders have different expectations about the
>>> start code that is prepended on H264 slices. Add a
>>> menu control to express the supported start code types
>>> (including no start code).
>>>
>>> Drivers are allowed to support only one start code type,
>>> but they can support both too.
>>>
>>> Note that this is independent of the H264 decoding mode,
>>> which specifies the granularity of the decoding operations.
>>> Either in frame-based or slice-based mode, this new control
>>> will allow to define the start code expected on H264 slices.
>>>
>>> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
>>> ---
>>> Changes in v4:
>>> * New patch.
>>> ---
>>> .../media/uapi/v4l/ext-ctrls-codec.rst | 31 +++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ++++++
>>> include/media/h264-ctrls.h | 6 ++++
>>> 3 files changed, 46 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
>>> index ea0455957149..94fd3a9b8b9e 100644
>>> --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
>>> +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
>>> @@ -2062,6 +2062,37 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>>> The OUTPUT buffer should contain all slices needed to decode the
>>> frame/field.
>>>
>>> +``V4L2_CID_MPEG_VIDEO_H264_STARTCODE (enum)``
>>> + Specifies the H264 slice start code expected for each slice.
>>> + This control shall e used to complement V4L2_PIX_FMT_H264_SLICE
>>
>> e -> be
>>
>>> + pixel format. Drivers may expose a single or multiple
>>> + start codes, depending on what they can support.
>>> +
>>> + .. note::
>>> +
>>> + This menu control is not yet part of the public kernel API and
>>> + it is expected to change.
>>> +
>>> +.. c:type:: v4l2_mpeg_video_h264_startcode
>>> +
>>> +.. cssclass:: longtable
>>> +
>>> +.. flat-table::
>>> + :header-rows: 0
>>> + :stub-columns: 0
>>> + :widths: 1 1 2
>>> +
>>> + * - ``V4L2_MPEG_VIDEO_H264_NO_STARTCODE``
>>> + - 0
>>> + - Selecting this value specifies that H264 slices are passed
>>> + to the driver without any start code.
>>> + Bla.
>>
>> Bla?
>>
>
> Well, despite how many times I reviewed this doc, it seems
> this slipped through :-(
>
>>> + * - ``V4L2_MPEG_VIDEO_H264_ANNEX_B_STARTCODE``
>>> + - 1
>>> + - Selecting this value specifies that H264 slices are expected
>>> + to be prefixed by Annex B start codes. According to :ref:`h264`
>>> + valid start codes can be 3-bytes 0x000001, or 4-bytes 0x00000001.
>>> +
>>
>> I had the impression that it is more common to require startcodes. If that's
>> indeed the case, shouldn't this have value 0 instead of 1?
>> 1?
>>
>
> I'm not sure this is indeed the case.
>
> For instance, VA-API accelerators work on H264 slices that are not
> prepended by the NALU start code.
>
> I'm fine flipping the values, but at this point, with only cedrus and hantro,
> there's doesn't seem to be a "natural" choice.

OK, let's keep this as-is then.

Regards,

Hans

>
> Thanks,
> Ezequiel
>