linux-kernel - RE: [EXT] Re: [PATCH] media: videobuf2: add V4L2_BUF_FLAG_HEADERS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AM6PR04MB63419629E384A48E4FFFCBFBE70E9@AM6PR04MB6341.eurprd04.prod.outlook.com>
Date:   Fri, 25 Nov 2022 01:48:44 +0000
From:   Ming Qian <ming.qian@....com>
To:     Hans Verkuil <hverkuil-cisco@...all.nl>,
        "mchehab@...nel.org" <mchehab@...nel.org>
CC:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Nicolas Dufresne <nicolas@...fresne.ca>,
        Tomasz Figa <tfiga@...omium.org>
Subject: RE: [EXT] Re: [PATCH] media: videobuf2: add
 V4L2_BUF_FLAG_HEADERS_ONLY flag

>From: Hans Verkuil <hverkuil-cisco@...all.nl>
>Sent: 2022年11月24日 18:42
>To: Ming Qian <ming.qian@....com>; mchehab@...nel.org
>Cc: shawnguo@...nel.org; robh+dt@...nel.org; s.hauer@...gutronix.de;
>kernel@...gutronix.de; festevam@...il.com; dl-linux-imx <linux-
>imx@....com>; linux-media@...r.kernel.org; linux-kernel@...r.kernel.org;
>linux-arm-kernel@...ts.infradead.org; Nicolas Dufresne
><nicolas@...fresne.ca>; Tomasz Figa <tfiga@...omium.org>
>Subject: [EXT] Re: [PATCH] media: videobuf2: add
>V4L2_BUF_FLAG_HEADERS_ONLY flag
>
>Caution: EXT Email
>
>+CC Nicolas and Tomasz.
>
>I would like some feedback for this patch.
>
>On 12/07/2022 11:37, Ming Qian wrote:
>> By setting the V4L2_BUF_FLAG_HEADERS_ONLY flag, hint the vb2 only
>> contains stream header, but does not contain any frame data.
>>
>> This flag needs to be used when header mode is set to
>> V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE.
>>
>> Signed-off-by: Ming Qian <ming.qian@....com>
>> ---
>>  Documentation/userspace-api/media/v4l/buffer.rst      | 11 +++++++++++
>>  .../userspace-api/media/v4l/ext-ctrls-codec.rst       | 10 +++++++---
>>  include/uapi/linux/videodev2.h                        |  2 ++
>>  3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst
>> b/Documentation/userspace-api/media/v4l/buffer.rst
>> index 4638ec64db00..18a6f5fcc822 100644
>> --- a/Documentation/userspace-api/media/v4l/buffer.rst
>> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
>> @@ -607,6 +607,17 @@ Buffer Flags
>>       the format. Any subsequent call to the
>>       :ref:`VIDIOC_DQBUF <VIDIOC_QBUF>` ioctl will not block anymore,
>>       but return an ``EPIPE`` error code.
>> +    * .. _`V4L2-BUF-FLAG-HEADERS-ONLY`:
>> +
>> +      - ``V4L2_BUF_FLAG_HEADERS_ONLY``
>> +      - 0x00200000
>> +      - This flag may be set when the buffer only contains codec
>
>Set by the driver or userspace? Or either, depending on whether it is an
>encoder or decoder?
>
>codec -> the codec
>
>> +     header, but does not contain any frame data. Usually the codec
>> +     header is merged to the next idr frame, with the flag
>
>to -> with
>idr -> IDR
>
>> +     ``V4L2_BUF_FLAG_KEYFRAME``, but there is still some scenes that
>> + will
>
>is -> are
>
>scenes: do you mean 'scenarios'?
>
>> +     split the header and queue it separately. This flag can set only
>> + when
>
>"split the header and queue it separately" -> queue the header in a separate
>buffer
>
>can -> can be
>
>> +     codec support V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE,
>
>codec -> the codec
>support -> supports
>
>> +     and the header mode is set to
>> + V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE
>>      * .. _`V4L2-BUF-FLAG-REQUEST-FD`:
>>
>>        - ``V4L2_BUF_FLAG_REQUEST_FD``
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index 6183f43f4d73..478b6af4205d 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -1386,8 +1386,12 @@ enum
>v4l2_mpeg_video_intra_refresh_period_type -
>>      (enum)
>>
>>  enum v4l2_mpeg_video_header_mode -
>> -    Determines whether the header is returned as the first buffer or is
>> -    it returned together with the first frame. Applicable to encoders.
>> +    Determines whether the header is returned as the first buffer
>> +    with flag V4L2_BUF_FLAG_HEADERS_ONLY or is
>
>or is it -> or if it is
>
>> +    it returned together with the first frame.
>> +    Applicable to encoders and decoders.
>> +    If it's not implemented in a driver,
>> +    V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME is to be
>> + assumed,
>>      Possible values are:
>>
>>  .. raw:: latex
>> @@ -1401,7 +1405,7 @@ enum v4l2_mpeg_video_header_mode -
>>      :stub-columns: 0
>>
>>      * - ``V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE``
>> -      - The stream header is returned separately in the first buffer.
>> +      - The stream header is returned separately in the first buffer with the
>flag V4L2_BUF_FLAG_HEADERS_ONLY.
>>      * - ``V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME``
>>        - The stream header is returned together with the first encoded
>>       frame.
>> diff --git a/include/uapi/linux/videodev2.h
>> b/include/uapi/linux/videodev2.h index 5311ac4fde35..6fd96acd6080
>> 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1131,6 +1131,8 @@ static inline __u64 v4l2_timeval_to_ns(const struct
>timeval *tv)
>>  #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE         0x00010000
>>  /* mem2mem encoder/decoder */
>>  #define V4L2_BUF_FLAG_LAST                   0x00100000
>> +/* Buffer only contains codec header */
>
>codec -> the codec
>
>> +#define V4L2_BUF_FLAG_HEADERS_ONLY           0x00200000
>>  /* request_fd is valid */
>>  #define V4L2_BUF_FLAG_REQUEST_FD             0x00800000
>>
>
>Of course, there needs to be a driver that uses this as well. And drivers that
>support V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE would need to add
>support for this flag as well, I guess.
>
>And what I haven't seen here is *why* you need this flag. There are already
>drivers that support V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE, and
>they managed fine without it.
>
>Regards,
>
>        Hans

Hi Hans,
   The V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE is only supported by encoder currently,
And I want to apply it to decoder too. As the user may queue the codec header in a separate buffer.
Especially in android case, but the amphion vpu requires one buffer contains one frame, if not, driver will merge the header to the next IDR frame. So we need this flag to handle such case.

   And for encoder, if the V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE is set, the stream header is returned separately in the first buffer. But there are some codecs that can produce sps, pps, vps separately, it means there may be more than 1 buffers returned with header data only. So I think this flag is also an enhancement for encoder.

Ming