Le mardi 05 juillet 2022 à 11:34 +0000, Ming Qian a écrit :

quoted

quoted

From: Ming Qian
Sent: 2022年7月5日 9:52
To: Nicolas Dufresne <redacted>; mchehab@kernel.org;
hverkuil-cisco@xs4all.nl
Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
[off-list ref]; linux-media@vger.kernel.org;
linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Subject: RE: [EXT] Re: [PATCH] media: videobuf2: add
V4L2_BUF_FLAG_CODECCONFIG flag

quoted

From: Nicolas Dufresne <redacted>
Sent: 2022年7月4日 23:53
To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
hverkuil-cisco@xs4all.nl
Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
[off-list ref]; linux-media@vger.kernel.org;
linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Subject: [EXT] Re: [PATCH] media: videobuf2: add
V4L2_BUF_FLAG_CODECCONFIG flag

Caution: EXT Email

Le mardi 28 juin 2022 à 10:19 +0800, Ming Qian a écrit :

quoted

By setting the V4L2_BUF_FLAG_CODECCONFIG flag, user-space should be
able to hint decoder the vb2 only contains codec config header, but
does not contain any frame data.
It's only used for parsing header, and can't be decoded.

This is copied from OMX specification. I think we we import this, we
should at least refer to the original.

Hi Nicolas,
   Do you mean OMX_BUFFERFLAG_CODECCONFIG?
   I'm sorry that I didn't notice it before.
   Currently we only encounter this requirement on Android, I'm not sure
if
it has a reference to omx.
   And thank you very much for pointing out it.

Android media stack has been based on OMX for the last decade. They are
slowly moving to CODEC2 which more or less is a similar abstraction with
similar ideas. Let's research prior art, so we don't screw compatibility.

I got it, I'll try to study the android codec2, 
and do you agree that we should add V4L2_BUF_FLAG_CODECCONFIG flag, just like
OMX_BUFFERFLAG_CODECCONFIG?
Or is there any other solution that can handle this case?

We can probably discuss the name. CODECCONFIG is a bit strange, could be
CODEC_HEADER, HEADER_ONLY, CONFIG_ONLY, something along these lines. I'm just
wondering what is the best rule, since more specification is needed here.
Current userland expect full frames into each encoded buffer. If we start
splitting these up, we'll break non-android userland (and Android userland does
not seems to be very upstream thing, every vendor forks it).

I also think that what the CODECCONFIG contains and its format need to be
strictly documented for every CODEC that would allow it. In H.264 notably, the
headers could be packed in Annex B. or AVCc NAL headers. If we look at FFMPEG,
which uses codec_data name, they only requires this when the header are not
"inline", which means only for AVCc. Also, many codec_data is for other codecs
get wrapped into ISOMP4 or Matroska (webm) envelope.

On the other hand, I don't remember if the 1 frame per buffer is an actual rule
or simply what existing userland expect. Also, I'll be fair, there is no reason
this must come from the driver. Android OMX or CODEC2 COMPONENT is a userland
component, it could do bitstream scanning (using traditional boyer-more) to find
and split appart the config to satisfy its internal API. This can be done with
low overhead and zero-copy.

quoted

quoted

Ming

quoted

quoted

Current, it's usually used by android.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 Documentation/userspace-api/media/v4l/buffer.rst | 9 +++++++++
 include/uapi/linux/videodev2.h                   | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/buffer.rst

b/Documentation/userspace-api/media/v4l/buffer.rst
index 4638ec64db00..acdc4556f4f4 100644

--- a/Documentation/userspace-api/media/v4l/buffer.rst
+++ b/Documentation/userspace-api/media/v4l/buffer.rst

@@ -607,6 +607,15 @@ 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-CODECCONFIG`:
+
+      - ``V4L2_BUF_FLAG_CODECCONFIG``
+      - 0x00200000
+      - This flag may be set when the buffer only contains codec
config
+    header, but does not contain any frame data. Usually the codec

config

quoted

quoted

+    header is merged to the next idr frame, with the flag
+    ``V4L2_BUF_FLAG_KEYFRAME``, but there is still some scenes that

will

quoted

quoted

+    split the header and queue it separately.

I think the documentation is clear. Now, if a driver uses this, will
existing userland (perhaps good to check GStreamer, FFMPEG and
Chromium ?) will break ?
So we need existing driver to do this when flagged to, and just
copy/append when the userland didn't opt-in that feature ?

quoted

     * .. _`V4L2-BUF-FLAG-REQUEST-FD`:

       - ``V4L2_BUF_FLAG_REQUEST_FD`` diff --git
a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5311ac4fde35..8708ef257710
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)

quoted

 #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE         0x00010000
 /* mem2mem encoder/decoder */
 #define V4L2_BUF_FLAG_LAST                   0x00100000
+/* Buffer only contains codec header */
+#define V4L2_BUF_FLAG_CODECCONFIG            0x00200000
 /* request_fd is valid */
 #define V4L2_BUF_FLAG_REQUEST_FD             0x00800000

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel