[PATCH 00/14] Stateful/stateless codec core support

[PATCH 00/14] Stateful/stateless codec core support

[Hans Verkuil]

This series consolidates various patches/patch series that add
features or document memory-to-memory video codec interfaces.

This includes patches adding V4L2_FMT_FLAG_DYN_RESOLUTION,
new code adding V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER, new code
adding V4L2_DEC_CMD_FLUSH, patches adding V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
(now with documentation) and patches documenting the stateful
encoder/decoder and stateless decoder.

The stateful encoder documentation is still RFC quality (there are
open TODOs, see https://patchwork.kernel.org/cover/10972783/).

The stateless decoder documentation is the same as the v5 posted
by Alexandre, but with my comments incorporated.

Also added are updated pixelformat descriptions. Please review this!
I didn't update the MPEG4 format since I'm not sure what to put there.

If anyone has access to recent codec standards, then I would really
like to have the right references to 'MPEG Picture' and 'Access Unit'.
It would be good to just refer to the definition of what a Picture
or Access Unit is in the right standards.

Changes for the stateful decoder documentation since v4:

- In the Decoding section change "multiple ``OUTPUT`` buffers generate
  one ``CAPTURE`` buffer: timestamp of the ``OUTPUT`` buffer queued last
  will be copied." to "queued first" since this corresponds to
  existing implementations.

- Document that width and height are required fields in step 4 of the
  Capture Setup.

- Mention the new ENUM_FMT flags.

Changes for the stateless decoder documentation since v5:

- Document that width and height are required fields in step 4 of the
  Capture Setup.

- Mention the new V4L2_DEC_CMD_FLUSH command to flush the last held
  capture buffer. This replaces the 'queue an empty buffer' solution.

In my view this series is ready to go in, except for the last patch
(stateful encoder).

Maxime, I didn't add the proposed V4L2_FMT_FLAG_MANUAL_RESOLUTION
flag since I think that can go in separately. I also am not 100%
happy with that name, although I can't think of a better one.

Regards,

	Hans

Alexandre Courbot (1):
  media: docs-rst: Document m2m stateless video decoder interface

Hans Verkuil (6):
  v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite
  videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER
  videodev2.h.rst.exceptions: tymecode -> timecode
  vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
  videodev2.h: add V4L2_DEC_CMD_FLUSH
  pixfmt-compressed.rst: improve H264/HEVC/MPEG1+2/VP8+9 documentation

Maxime Jourdan (5):
  videodev2.h: add V4L2_FMT_FLAG_DYN_RESOLUTION
  media: venus: vdec: flag OUTPUT formats with
    V4L2_FMT_FLAG_DYN_RESOLUTION
  media: s5p_mfc_dec: set flags for OUTPUT coded formats
  media: mtk-vcodec: flag OUTPUT formats with
    V4L2_FMT_FLAG_DYN_RESOLUTION
  media: vicodec: set flags for vdec/stateful OUTPUT coded formats

Tomasz Figa (2):
  media: docs-rst: Document memory-to-memory video decoder interface
  media: docs-rst: Document memory-to-memory video encoder interface

 Documentation/media/uapi/v4l/buffer.rst       |   13 +
 Documentation/media/uapi/v4l/dev-decoder.rst  | 1101 +++++++++++++++++
 Documentation/media/uapi/v4l/dev-encoder.rst  |  608 +++++++++
 Documentation/media/uapi/v4l/dev-mem2mem.rst  |   10 +-
 .../media/uapi/v4l/dev-stateless-decoder.rst  |  424 +++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |   36 +-
 Documentation/media/uapi/v4l/pixfmt-v4l2.rst  |   10 +
 Documentation/media/uapi/v4l/v4l2.rst         |   12 +-
 .../media/uapi/v4l/vidioc-decoder-cmd.rst     |   52 +-
 .../media/uapi/v4l/vidioc-dqevent.rst         |   11 +-
 .../media/uapi/v4l/vidioc-encoder-cmd.rst     |   51 +-
 .../media/uapi/v4l/vidioc-enum-fmt.rst        |   16 +
 .../media/uapi/v4l/vidioc-reqbufs.rst         |    6 +
 .../media/videodev2.h.rst.exceptions          |    7 +-
 .../media/common/videobuf2/videobuf2-v4l2.c   |    8 +-
 .../platform/mtk-vcodec/mtk_vcodec_dec.c      |    4 +
 .../platform/mtk-vcodec/mtk_vcodec_drv.h      |    1 +
 drivers/media/platform/qcom/venus/core.h      |    1 +
 drivers/media/platform/qcom/venus/vdec.c      |   11 +
 .../media/platform/s5p-mfc/s5p_mfc_common.h   |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |   18 +
 drivers/media/platform/vicodec/vicodec-core.c |    3 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |    2 +-
 include/media/v4l2-mem2mem.h                  |   42 +
 include/media/videobuf2-core.h                |    3 +
 include/media/videobuf2-v4l2.h                |    5 +
 include/uapi/linux/videodev2.h                |   20 +-
 27 files changed, 2419 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/dev-decoder.rst
 create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst
 create mode 100644 Documentation/media/uapi/v4l/dev-stateless-decoder.rst

-- 
2.20.1

[PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Hans Verkuil]

If a driver sets a FMT flag in the enum_fmt op, then that will be
ignored since v4l_fill_fmtdesc() overwrites it again.

v4l_fill_fmtdesc() should OR its flag, not overwrite it.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 80efc581e3f9..911a20f915c5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
 static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-- 
2.20.1

[PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Hans Verkuil]

Add an enum_fmt format flag to specifically tag coded formats where
full bitstream parsing is supported by the device.

Some stateful decoders are capable of fully parsing a bitstream,
but others require that userspace pre-parses the bitstream into
frames or fields (see the corresponding pixelformat descriptions
for details).

If this flag is set, then this pre-parsing step is not required
(but still possible, of course).

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
 Documentation/media/videodev2.h.rst.exceptions   | 1 +
 include/uapi/linux/videodev2.h                   | 5 +++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
index 822d6730e7d2..4e24e671f32e 100644
--- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
@@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
       - This format is not native to the device but emulated through
 	software (usually libv4l2), where possible try to use a native
 	format instead for better performance.
+    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
+      - 0x0004
+      - The hardware decoder for this compressed bitstream format (aka coded
+	format) is capable of parsing the bitstream. Applications do not
+	need to parse the bitstream themselves to find the boundaries between
+	frames/fields. This flag can only be used in combination with the
+	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
+	formats only.
 
 
 Return Value
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index 55cbe324b9fc..74fb9f00c12d 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -180,6 +180,7 @@ replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
 # V4L2 format flags
 replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
 replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
+replace define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER fmtdesc-flags
 
 # V4L2 tymecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2427bc4d8eba..8c5a28666b16 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -774,8 +774,9 @@ struct v4l2_fmtdesc {
 	__u32		    reserved[4];
 };
 
-#define V4L2_FMT_FLAG_COMPRESSED 0x0001
-#define V4L2_FMT_FLAG_EMULATED   0x0002
+#define V4L2_FMT_FLAG_COMPRESSED		0x0001
+#define V4L2_FMT_FLAG_EMULATED			0x0002
+#define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER	0x0004
 
 	/* Frame Size and frame rate enumeration */
 /*
-- 
2.20.1

[PATCH 03/14] videodev2.h: add V4L2_FMT_FLAG_DYN_RESOLUTION

[Hans Verkuil]

From: Maxime Jourdan <mjourdan@baylibre.com>

Add an enum_fmt format flag to specifically tag coded formats where
dynamic resolution switching is supported by the device.

This is useful for some codec drivers that can support dynamic
resolution switching for one or more of their listed coded formats. It
allows userspace to know whether it should extract the video parameters
itself, or if it can rely on the device to send V4L2_EVENT_SOURCE_CHANGE
when such changes are detected.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
[hverkuil-cisco@xs4all.nl: added flag to videodev2.h.rst.exceptions]
[hverkuil-cisco@xs4all.nl: updated commit text: 'one or more' instead of 'all']
Acked-by: Tomasz Figa <tfiga@chromium.org>
---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
 Documentation/media/videodev2.h.rst.exceptions   | 1 +
 include/uapi/linux/videodev2.h                   | 1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
index 4e24e671f32e..05454780cb21 100644
--- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
@@ -135,6 +135,14 @@ one until ``EINVAL`` is returned.
 	frames/fields. This flag can only be used in combination with the
 	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
 	formats only.
+    * - ``V4L2_FMT_FLAG_DYN_RESOLUTION``
+      - 0x0008
+      - Dynamic resolution switching is supported by the device for this
+	compressed bitstream format (aka coded format). It will notify the user
+	via the event ``V4L2_EVENT_SOURCE_CHANGE`` when changes in the video
+	parameters are detected. This flag can only be used in combination
+	with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
+	compressed formats only.
 
 
 Return Value
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index 74fb9f00c12d..0a9a1b386443 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -181,6 +181,7 @@ replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
 replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
 replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
 replace define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER fmtdesc-flags
+replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
 
 # V4L2 tymecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 8c5a28666b16..ed572b05bd25 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -777,6 +777,7 @@ struct v4l2_fmtdesc {
 #define V4L2_FMT_FLAG_COMPRESSED		0x0001
 #define V4L2_FMT_FLAG_EMULATED			0x0002
 #define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER	0x0004
+#define V4L2_FMT_FLAG_DYN_RESOLUTION		0x0008
 
 	/* Frame Size and frame rate enumeration */
 /*
-- 
2.20.1

[PATCH 04/14] videodev2.h.rst.exceptions: tymecode -> timecode

[Hans Verkuil]

Fix typo.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/media/videodev2.h.rst.exceptions | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index 0a9a1b386443..b6cb9fa6c8a8 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -183,14 +183,14 @@ replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
 replace define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER fmtdesc-flags
 replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
 
-# V4L2 tymecode types
+# V4L2 timecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
 replace define V4L2_TC_TYPE_25FPS timecode-type
 replace define V4L2_TC_TYPE_30FPS timecode-type
 replace define V4L2_TC_TYPE_50FPS timecode-type
 replace define V4L2_TC_TYPE_60FPS timecode-type
 
-# V4L2 tymecode flags
+# V4L2 timecode flags
 replace define V4L2_TC_FLAG_DROPFRAME timecode-flags
 replace define V4L2_TC_FLAG_COLORFRAME timecode-flags
 replace define V4L2_TC_USERBITS_field timecode-flags
-- 
2.20.1

[PATCH 05/14] media: venus: vdec: flag OUTPUT formats with V4L2_FMT_FLAG_DYN_RESOLUTION

[Hans Verkuil]

From: Maxime Jourdan <mjourdan@baylibre.com>

Tag all the coded formats where the venus vdec supports dynamic
resolution switching.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/qcom/venus/core.h |  1 +
 drivers/media/platform/qcom/venus/vdec.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 9ab95fd57760..6c243309df4b 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -46,6 +46,7 @@ struct venus_format {
 	u32 pixfmt;
 	unsigned int num_planes;
 	u32 type;
+	u32 flags;
 };
 
 #define MAX_PLANES		4
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index e1f998656c07..380e8d1682e2 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -37,42 +37,52 @@ static const struct venus_format vdec_formats[] = {
 		.pixfmt = V4L2_PIX_FMT_MPEG4,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_MPEG2,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_H263,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_H264,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_VP8,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_VP9,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_XVID,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	}, {
 		.pixfmt = V4L2_PIX_FMT_HEVC,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 };
 
@@ -351,6 +361,7 @@ static int vdec_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
 		return -EINVAL;
 
 	f->pixelformat = fmt->pixfmt;
+	f->flags = fmt->flags;
 
 	return 0;
 }
-- 
2.20.1

[PATCH 06/14] media: s5p_mfc_dec: set flags for OUTPUT coded formats

[Hans Verkuil]

From: Maxime Jourdan <mjourdan@baylibre.com>

Tag all the coded formats where the s5p_mfc decoder supports dynamic
resolution switching or has a bitstream parser.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
[hverkuil-cisco@xs4all.nl: added HAS_BITSTREAM_PARSER]
---
 .../media/platform/s5p-mfc/s5p_mfc_common.h    |  1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 96d1ecd1521b..31b133af91eb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -723,6 +723,7 @@ struct s5p_mfc_fmt {
 	enum s5p_mfc_fmt_type type;
 	u32 num_planes;
 	u32 versions;
+	u32 flags;
 };
 
 /**
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 61e144a35201..2d45a4d8d536 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -62,6 +62,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_H264_MVC,
@@ -69,6 +71,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V6PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_H263,
@@ -76,6 +80,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_MPEG1,
@@ -83,6 +88,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_MPEG2,
@@ -90,6 +97,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_MPEG4,
@@ -97,6 +106,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_XVID,
@@ -104,6 +115,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_VC1_ANNEX_G,
@@ -111,6 +123,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_VC1_ANNEX_L,
@@ -118,6 +131,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V5PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_VP8,
@@ -125,6 +139,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V6PLUS_BITS,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_HEVC,
@@ -132,6 +147,8 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V10_BIT,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION |
+				  V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_VP9,
@@ -139,6 +156,7 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_DEC,
 		.num_planes	= 1,
 		.versions	= MFC_V10_BIT,
+		.flags		= V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
 };
 
-- 
2.20.1

Re: [PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Philipp Zabel]

On Wed, 2019-07-24 at 13:05 +0200, Hans Verkuil wrote:
> If a driver sets a FMT flag in the enum_fmt op, then that will be
> ignored since v4l_fill_fmtdesc() overwrites it again.
> 
> v4l_fill_fmtdesc() should OR its flag, not overwrite it.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 80efc581e3f9..911a20f915c5 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  
>  	if (descr)
>  		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
> -	fmt->flags = flags;
> +	fmt->flags |= flags;
>  }
>  
>  static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,

If the enum_fmt op does not write fmt->flags, will it not contain the
value provided by userspace at this point? I think p->flags must be
cleared in v4l2_enum_fmt() with this change, before the enum_fmt op is
called.

regards
Philipp

Re: [PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Hans Verkuil]

On 7/24/19 3:22 PM, Philipp Zabel wrote:
> On Wed, 2019-07-24 at 13:05 +0200, Hans Verkuil wrote:
>> If a driver sets a FMT flag in the enum_fmt op, then that will be
>> ignored since v4l_fill_fmtdesc() overwrites it again.
>>
>> v4l_fill_fmtdesc() should OR its flag, not overwrite it.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 80efc581e3f9..911a20f915c5 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>  
>>  	if (descr)
>>  		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
>> -	fmt->flags = flags;
>> +	fmt->flags |= flags;
>>  }
>>  
>>  static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
> 
> If the enum_fmt op does not write fmt->flags, will it not contain the
> value provided by userspace at this point? I think p->flags must be
> cleared in v4l2_enum_fmt() with this change, before the enum_fmt op is
> called.

All fields after 'type' in struct v4l2_fmtdesc are cleared by the core:
search for INFO_FL_CLEAR(v4l2_fmtdesc, type) in v4l2-ioctl.c.

So 'flags' is already zeroed when this function is called.

Regards,

	Hans

Re: [PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Philipp Zabel]

On Wed, 2019-07-24 at 15:30 +0200, Hans Verkuil wrote:
> On 7/24/19 3:22 PM, Philipp Zabel wrote:
> > On Wed, 2019-07-24 at 13:05 +0200, Hans Verkuil wrote:
> > > If a driver sets a FMT flag in the enum_fmt op, then that will be
> > > ignored since v4l_fill_fmtdesc() overwrites it again.
> > > 
> > > v4l_fill_fmtdesc() should OR its flag, not overwrite it.
> > > 
> > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > index 80efc581e3f9..911a20f915c5 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > @@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > >  
> > >  	if (descr)
> > >  		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
> > > -	fmt->flags = flags;
> > > +	fmt->flags |= flags;
> > >  }
> > >  
> > >  static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
> > 
> > If the enum_fmt op does not write fmt->flags, will it not contain the
> > value provided by userspace at this point? I think p->flags must be
> > cleared in v4l2_enum_fmt() with this change, before the enum_fmt op is
> > called.
> 
> All fields after 'type' in struct v4l2_fmtdesc are cleared by the core:
> search for INFO_FL_CLEAR(v4l2_fmtdesc, type) in v4l2-ioctl.c.
> 
> So 'flags' is already zeroed when this function is called.

Got it, thanks. In that case,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH 00/14] Stateful/stateless codec core support

[Hans Verkuil]

Just a reminder: due to problems with my provider (overly strict spammer
detection) I had to post this series again, this time without any CCs.

So the full series is here:

https://patchwork.linuxtv.org/cover/57699/

Please review. I certainly need help with "pixfmt-compressed.rst:
improve H264/HEVC/MPEG1+2/VP8+9 documentation".

Also, I only added V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER to the s5p-mfc and
vicodec drivers. Please comment if other drivers also support this.

Regards,

	Hans


On 7/24/19 1:05 PM, Hans Verkuil wrote:
> This series consolidates various patches/patch series that add
> features or document memory-to-memory video codec interfaces.
> 
> This includes patches adding V4L2_FMT_FLAG_DYN_RESOLUTION,
> new code adding V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER, new code
> adding V4L2_DEC_CMD_FLUSH, patches adding V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
> (now with documentation) and patches documenting the stateful
> encoder/decoder and stateless decoder.
> 
> The stateful encoder documentation is still RFC quality (there are
> open TODOs, see https://patchwork.kernel.org/cover/10972783/).
> 
> The stateless decoder documentation is the same as the v5 posted
> by Alexandre, but with my comments incorporated.
> 
> Also added are updated pixelformat descriptions. Please review this!
> I didn't update the MPEG4 format since I'm not sure what to put there.
> 
> If anyone has access to recent codec standards, then I would really
> like to have the right references to 'MPEG Picture' and 'Access Unit'.
> It would be good to just refer to the definition of what a Picture
> or Access Unit is in the right standards.
> 
> Changes for the stateful decoder documentation since v4:
> 
> - In the Decoding section change "multiple ``OUTPUT`` buffers generate
>   one ``CAPTURE`` buffer: timestamp of the ``OUTPUT`` buffer queued last
>   will be copied." to "queued first" since this corresponds to
>   existing implementations.
> 
> - Document that width and height are required fields in step 4 of the
>   Capture Setup.
> 
> - Mention the new ENUM_FMT flags.
> 
> Changes for the stateless decoder documentation since v5:
> 
> - Document that width and height are required fields in step 4 of the
>   Capture Setup.
> 
> - Mention the new V4L2_DEC_CMD_FLUSH command to flush the last held
>   capture buffer. This replaces the 'queue an empty buffer' solution.
> 
> In my view this series is ready to go in, except for the last patch
> (stateful encoder).
> 
> Maxime, I didn't add the proposed V4L2_FMT_FLAG_MANUAL_RESOLUTION
> flag since I think that can go in separately. I also am not 100%
> happy with that name, although I can't think of a better one.
> 
> Regards,
> 
> 	Hans
> 
> Alexandre Courbot (1):
>   media: docs-rst: Document m2m stateless video decoder interface
> 
> Hans Verkuil (6):
>   v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite
>   videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER
>   videodev2.h.rst.exceptions: tymecode -> timecode
>   vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
>   videodev2.h: add V4L2_DEC_CMD_FLUSH
>   pixfmt-compressed.rst: improve H264/HEVC/MPEG1+2/VP8+9 documentation
> 
> Maxime Jourdan (5):
>   videodev2.h: add V4L2_FMT_FLAG_DYN_RESOLUTION
>   media: venus: vdec: flag OUTPUT formats with
>     V4L2_FMT_FLAG_DYN_RESOLUTION
>   media: s5p_mfc_dec: set flags for OUTPUT coded formats
>   media: mtk-vcodec: flag OUTPUT formats with
>     V4L2_FMT_FLAG_DYN_RESOLUTION
>   media: vicodec: set flags for vdec/stateful OUTPUT coded formats
> 
> Tomasz Figa (2):
>   media: docs-rst: Document memory-to-memory video decoder interface
>   media: docs-rst: Document memory-to-memory video encoder interface
> 
>  Documentation/media/uapi/v4l/buffer.rst       |   13 +
>  Documentation/media/uapi/v4l/dev-decoder.rst  | 1101 +++++++++++++++++
>  Documentation/media/uapi/v4l/dev-encoder.rst  |  608 +++++++++
>  Documentation/media/uapi/v4l/dev-mem2mem.rst  |   10 +-
>  .../media/uapi/v4l/dev-stateless-decoder.rst  |  424 +++++++
>  .../media/uapi/v4l/pixfmt-compressed.rst      |   36 +-
>  Documentation/media/uapi/v4l/pixfmt-v4l2.rst  |   10 +
>  Documentation/media/uapi/v4l/v4l2.rst         |   12 +-
>  .../media/uapi/v4l/vidioc-decoder-cmd.rst     |   52 +-
>  .../media/uapi/v4l/vidioc-dqevent.rst         |   11 +-
>  .../media/uapi/v4l/vidioc-encoder-cmd.rst     |   51 +-
>  .../media/uapi/v4l/vidioc-enum-fmt.rst        |   16 +
>  .../media/uapi/v4l/vidioc-reqbufs.rst         |    6 +
>  .../media/videodev2.h.rst.exceptions          |    7 +-
>  .../media/common/videobuf2/videobuf2-v4l2.c   |    8 +-
>  .../platform/mtk-vcodec/mtk_vcodec_dec.c      |    4 +
>  .../platform/mtk-vcodec/mtk_vcodec_drv.h      |    1 +
>  drivers/media/platform/qcom/venus/core.h      |    1 +
>  drivers/media/platform/qcom/venus/vdec.c      |   11 +
>  .../media/platform/s5p-mfc/s5p_mfc_common.h   |    1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |   18 +
>  drivers/media/platform/vicodec/vicodec-core.c |    3 +
>  drivers/media/v4l2-core/v4l2-ioctl.c          |    2 +-
>  include/media/v4l2-mem2mem.h                  |   42 +
>  include/media/videobuf2-core.h                |    3 +
>  include/media/videobuf2-v4l2.h                |    5 +
>  include/uapi/linux/videodev2.h                |   20 +-
>  27 files changed, 2419 insertions(+), 57 deletions(-)
>  create mode 100644 Documentation/media/uapi/v4l/dev-decoder.rst
>  create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst
>  create mode 100644 Documentation/media/uapi/v4l/dev-stateless-decoder.rst
> 

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Paul Kocialkowski]

Hi,

On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> Add an enum_fmt format flag to specifically tag coded formats where
> full bitstream parsing is supported by the device.
> 
> Some stateful decoders are capable of fully parsing a bitstream,
> but others require that userspace pre-parses the bitstream into
> frames or fields (see the corresponding pixelformat descriptions
> for details).
> 
> If this flag is set, then this pre-parsing step is not required
> (but still possible, of course).
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
>  Documentation/media/videodev2.h.rst.exceptions   | 1 +
>  include/uapi/linux/videodev2.h                   | 5 +++--
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> index 822d6730e7d2..4e24e671f32e 100644
> --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
>        - This format is not native to the device but emulated through
>  	software (usually libv4l2), where possible try to use a native
>  	format instead for better performance.
> +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> +      - 0x0004
> +      - The hardware decoder for this compressed bitstream format (aka coded
> +	format) is capable of parsing the bitstream. Applications do not
> +	need to parse the bitstream themselves to find the boundaries between
> +	frames/fields. This flag can only be used in combination with the
> +	``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> +	formats only.

Should this flag be set for stateless codecs as well? It seems a bit over-kill
for this case. I am not sure whether "compressed bitstream format" clearly only
covers the formats used by stateful decoders and not the ones for stateless
decoders.

Cheers,

Paul

>  
>  Return Value
> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
> index 55cbe324b9fc..74fb9f00c12d 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -180,6 +180,7 @@ replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
>  # V4L2 format flags
>  replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
> +replace define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER fmtdesc-flags
>  
>  # V4L2 tymecode types
>  replace define V4L2_TC_TYPE_24FPS timecode-type
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 2427bc4d8eba..8c5a28666b16 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -774,8 +774,9 @@ struct v4l2_fmtdesc {
>  	__u32		    reserved[4];
>  };
>  
> -#define V4L2_FMT_FLAG_COMPRESSED 0x0001
> -#define V4L2_FMT_FLAG_EMULATED   0x0002
> +#define V4L2_FMT_FLAG_COMPRESSED		0x0001
> +#define V4L2_FMT_FLAG_EMULATED			0x0002
> +#define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER	0x0004
>  
>  	/* Frame Size and frame rate enumeration */
>  /*
> -- 
> 2.20.1
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH 04/14] videodev2.h.rst.exceptions: tymecode -> timecode

[Paul Kocialkowski]

Hi,

On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> Fix typo.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Looks good!

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  Documentation/media/videodev2.h.rst.exceptions | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
> index 0a9a1b386443..b6cb9fa6c8a8 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -183,14 +183,14 @@ replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
>  replace define V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER fmtdesc-flags
>  replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
>  
> -# V4L2 tymecode types
> +# V4L2 timecode types
>  replace define V4L2_TC_TYPE_24FPS timecode-type
>  replace define V4L2_TC_TYPE_25FPS timecode-type
>  replace define V4L2_TC_TYPE_30FPS timecode-type
>  replace define V4L2_TC_TYPE_50FPS timecode-type
>  replace define V4L2_TC_TYPE_60FPS timecode-type
>  
> -# V4L2 tymecode flags
> +# V4L2 timecode flags
>  replace define V4L2_TC_FLAG_DROPFRAME timecode-flags
>  replace define V4L2_TC_FLAG_COLORFRAME timecode-flags
>  replace define V4L2_TC_USERBITS_field timecode-flags
> -- 
> 2.20.1
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Tomasz Figa]

On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> > Add an enum_fmt format flag to specifically tag coded formats where
> > full bitstream parsing is supported by the device.
> >
> > Some stateful decoders are capable of fully parsing a bitstream,
> > but others require that userspace pre-parses the bitstream into
> > frames or fields (see the corresponding pixelformat descriptions
> > for details).
> >
> > If this flag is set, then this pre-parsing step is not required
> > (but still possible, of course).
> >
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
> >  Documentation/media/videodev2.h.rst.exceptions   | 1 +
> >  include/uapi/linux/videodev2.h                   | 5 +++--
> >  3 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > index 822d6730e7d2..4e24e671f32e 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
> >        - This format is not native to the device but emulated through
> >       software (usually libv4l2), where possible try to use a native
> >       format instead for better performance.
> > +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> > +      - 0x0004
> > +      - The hardware decoder for this compressed bitstream format (aka coded
> > +     format) is capable of parsing the bitstream. Applications do not
> > +     need to parse the bitstream themselves to find the boundaries between
> > +     frames/fields. This flag can only be used in combination with the
> > +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> > +     formats only.
>
> Should this flag be set for stateless codecs as well? It seems a bit over-kill
> for this case. I am not sure whether "compressed bitstream format" clearly only
> covers the formats used by stateful decoders and not the ones for stateless
> decoders.

I'd suggest using a different name for the flag, because "bitstream
parser" is actually one of the core differences between stateful and
stateless. All stateful decoders have bitstream parsers, the only
difference between the implementations is the unit on which the parser
operates, i.e. full stream, frame, NALU.

Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
framed/sliced chunks)?

Regardless of that, it doesn't make sense for a stateless decoder to
set this flag anyway, because the userspace needs to parse the whole
stream anyway and the whole stateless API is based on the assumption
that the userspace splits the bitstream into frames (or slices).

Best regards,
Tomasz

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Paul Kocialkowski]

Hi,

On Sun 28 Jul 19, 23:05, Tomasz Figa wrote:
> On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> > > Add an enum_fmt format flag to specifically tag coded formats where
> > > full bitstream parsing is supported by the device.
> > >
> > > Some stateful decoders are capable of fully parsing a bitstream,
> > > but others require that userspace pre-parses the bitstream into
> > > frames or fields (see the corresponding pixelformat descriptions
> > > for details).
> > >
> > > If this flag is set, then this pre-parsing step is not required
> > > (but still possible, of course).
> > >
> > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > ---
> > >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
> > >  Documentation/media/videodev2.h.rst.exceptions   | 1 +
> > >  include/uapi/linux/videodev2.h                   | 5 +++--
> > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > index 822d6730e7d2..4e24e671f32e 100644
> > > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
> > >        - This format is not native to the device but emulated through
> > >       software (usually libv4l2), where possible try to use a native
> > >       format instead for better performance.
> > > +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> > > +      - 0x0004
> > > +      - The hardware decoder for this compressed bitstream format (aka coded
> > > +     format) is capable of parsing the bitstream. Applications do not
> > > +     need to parse the bitstream themselves to find the boundaries between
> > > +     frames/fields. This flag can only be used in combination with the
> > > +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> > > +     formats only.
> >
> > Should this flag be set for stateless codecs as well? It seems a bit over-kill
> > for this case. I am not sure whether "compressed bitstream format" clearly only
> > covers the formats used by stateful decoders and not the ones for stateless
> > decoders.
> 
> I'd suggest using a different name for the flag, because "bitstream
> parser" is actually one of the core differences between stateful and
> stateless. All stateful decoders have bitstream parsers, the only
> difference between the implementations is the unit on which the parser
> operates, i.e. full stream, frame, NALU.
> 
> Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
> framed/sliced chunks)?

Sure, that seems like a more explicit name regarding what it's supposed to
describe in my opinion.

> Regardless of that, it doesn't make sense for a stateless decoder to
> set this flag anyway, because the userspace needs to parse the whole
> stream anyway and the whole stateless API is based on the assumption
> that the userspace splits the bitstream into frames (or slices).

Indeed, I agree that it doesn't make sense, but I thought that the name of the
flag could be confusing. Since there is no direct equivalency between
"stateless" and "doesn't parse the bitstream" (as we've seen with the rockchip
decoder needing to parse the slice header on its own), that could have been
ambiguous. I think the name you're suggesting mostly solves this concern.

I'm still a bit unsure about what "compressed formats" entails or not, so it
could be good to explicitly mention that this applies to stateful decoders only
(but it's just a suggestion, advanced users of the API will probably find it
straightforward).

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Tomasz Figa]

On Mon, Jul 29, 2019 at 10:12 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Sun 28 Jul 19, 23:05, Tomasz Figa wrote:
> > On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> > > > Add an enum_fmt format flag to specifically tag coded formats where
> > > > full bitstream parsing is supported by the device.
> > > >
> > > > Some stateful decoders are capable of fully parsing a bitstream,
> > > > but others require that userspace pre-parses the bitstream into
> > > > frames or fields (see the corresponding pixelformat descriptions
> > > > for details).
> > > >
> > > > If this flag is set, then this pre-parsing step is not required
> > > > (but still possible, of course).
> > > >
> > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > ---
> > > >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
> > > >  Documentation/media/videodev2.h.rst.exceptions   | 1 +
> > > >  include/uapi/linux/videodev2.h                   | 5 +++--
> > > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > index 822d6730e7d2..4e24e671f32e 100644
> > > > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
> > > >        - This format is not native to the device but emulated through
> > > >       software (usually libv4l2), where possible try to use a native
> > > >       format instead for better performance.
> > > > +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> > > > +      - 0x0004
> > > > +      - The hardware decoder for this compressed bitstream format (aka coded
> > > > +     format) is capable of parsing the bitstream. Applications do not
> > > > +     need to parse the bitstream themselves to find the boundaries between
> > > > +     frames/fields. This flag can only be used in combination with the
> > > > +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> > > > +     formats only.
> > >
> > > Should this flag be set for stateless codecs as well? It seems a bit over-kill
> > > for this case. I am not sure whether "compressed bitstream format" clearly only
> > > covers the formats used by stateful decoders and not the ones for stateless
> > > decoders.
> >
> > I'd suggest using a different name for the flag, because "bitstream
> > parser" is actually one of the core differences between stateful and
> > stateless. All stateful decoders have bitstream parsers, the only
> > difference between the implementations is the unit on which the parser
> > operates, i.e. full stream, frame, NALU.
> >
> > Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
> > framed/sliced chunks)?
>
> Sure, that seems like a more explicit name regarding what it's supposed to
> describe in my opinion.
>
> > Regardless of that, it doesn't make sense for a stateless decoder to
> > set this flag anyway, because the userspace needs to parse the whole
> > stream anyway and the whole stateless API is based on the assumption
> > that the userspace splits the bitstream into frames (or slices).
>
> Indeed, I agree that it doesn't make sense, but I thought that the name of the
> flag could be confusing. Since there is no direct equivalency between
> "stateless" and "doesn't parse the bitstream" (as we've seen with the rockchip
> decoder needing to parse the slice header on its own), that could have been
> ambiguous. I think the name you're suggesting mostly solves this concern.
>
> I'm still a bit unsure about what "compressed formats" entails or not, so it
> could be good to explicitly mention that this applies to stateful decoders only
> (but it's just a suggestion, advanced users of the API will probably find it
> straightforward).

My understanding is that a compressed format is any format that
doesn't have a directly accessible 2D pixel matrix in its memory
representation, so all the bitstream formats should have it set.

Best regards,
Tomasz

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Hans Verkuil]

On 7/29/19 3:18 PM, Tomasz Figa wrote:
> On Mon, Jul 29, 2019 at 10:12 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
>>
>> Hi,
>>
>> On Sun 28 Jul 19, 23:05, Tomasz Figa wrote:
>>> On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
>>> <paul.kocialkowski@bootlin.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
>>>>> Add an enum_fmt format flag to specifically tag coded formats where
>>>>> full bitstream parsing is supported by the device.
>>>>>
>>>>> Some stateful decoders are capable of fully parsing a bitstream,
>>>>> but others require that userspace pre-parses the bitstream into
>>>>> frames or fields (see the corresponding pixelformat descriptions
>>>>> for details).
>>>>>
>>>>> If this flag is set, then this pre-parsing step is not required
>>>>> (but still possible, of course).
>>>>>
>>>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>>> ---
>>>>>  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
>>>>>  Documentation/media/videodev2.h.rst.exceptions   | 1 +
>>>>>  include/uapi/linux/videodev2.h                   | 5 +++--
>>>>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
>>>>> index 822d6730e7d2..4e24e671f32e 100644
>>>>> --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
>>>>> +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
>>>>> @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
>>>>>        - This format is not native to the device but emulated through
>>>>>       software (usually libv4l2), where possible try to use a native
>>>>>       format instead for better performance.
>>>>> +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
>>>>> +      - 0x0004
>>>>> +      - The hardware decoder for this compressed bitstream format (aka coded
>>>>> +     format) is capable of parsing the bitstream. Applications do not
>>>>> +     need to parse the bitstream themselves to find the boundaries between
>>>>> +     frames/fields. This flag can only be used in combination with the
>>>>> +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
>>>>> +     formats only.
>>>>
>>>> Should this flag be set for stateless codecs as well? It seems a bit over-kill
>>>> for this case. I am not sure whether "compressed bitstream format" clearly only
>>>> covers the formats used by stateful decoders and not the ones for stateless
>>>> decoders.
>>>
>>> I'd suggest using a different name for the flag, because "bitstream
>>> parser" is actually one of the core differences between stateful and
>>> stateless. All stateful decoders have bitstream parsers, the only
>>> difference between the implementations is the unit on which the parser
>>> operates, i.e. full stream, frame, NALU.
>>>
>>> Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
>>> framed/sliced chunks)?
>>
>> Sure, that seems like a more explicit name regarding what it's supposed to
>> describe in my opinion.

I like that name. And this flag is valid for stateful decoders only.

>>
>>> Regardless of that, it doesn't make sense for a stateless decoder to
>>> set this flag anyway, because the userspace needs to parse the whole
>>> stream anyway and the whole stateless API is based on the assumption
>>> that the userspace splits the bitstream into frames (or slices).
>>
>> Indeed, I agree that it doesn't make sense, but I thought that the name of the
>> flag could be confusing. Since there is no direct equivalency between
>> "stateless" and "doesn't parse the bitstream" (as we've seen with the rockchip
>> decoder needing to parse the slice header on its own), that could have been
>> ambiguous. I think the name you're suggesting mostly solves this concern.
>>
>> I'm still a bit unsure about what "compressed formats" entails or not, so it
>> could be good to explicitly mention that this applies to stateful decoders only
>> (but it's just a suggestion, advanced users of the API will probably find it
>> straightforward).
> 
> My understanding is that a compressed format is any format that
> doesn't have a directly accessible 2D pixel matrix in its memory
> representation, so all the bitstream formats should have it set.

Correct.

Regards,

	Hans

> 
> Best regards,
> Tomasz
> 

Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 5228 bytes --]

Le mardi 30 juillet 2019 à 09:21 +0200, Hans Verkuil a écrit :
> On 7/29/19 3:18 PM, Tomasz Figa wrote:
> > On Mon, Jul 29, 2019 at 10:12 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > > 
> > > On Sun 28 Jul 19, 23:05, Tomasz Figa wrote:
> > > > On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> > > > > > Add an enum_fmt format flag to specifically tag coded formats where
> > > > > > full bitstream parsing is supported by the device.
> > > > > > 
> > > > > > Some stateful decoders are capable of fully parsing a bitstream,
> > > > > > but others require that userspace pre-parses the bitstream into
> > > > > > frames or fields (see the corresponding pixelformat descriptions
> > > > > > for details).
> > > > > > 
> > > > > > If this flag is set, then this pre-parsing step is not required
> > > > > > (but still possible, of course).
> > > > > > 
> > > > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > > > > > ---
> > > > > >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
> > > > > >  Documentation/media/videodev2.h.rst.exceptions   | 1 +
> > > > > >  include/uapi/linux/videodev2.h                   | 5 +++--
> > > > > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > > > index 822d6730e7d2..4e24e671f32e 100644
> > > > > > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > > > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > > > > @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
> > > > > >        - This format is not native to the device but emulated through
> > > > > >       software (usually libv4l2), where possible try to use a native
> > > > > >       format instead for better performance.
> > > > > > +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> > > > > > +      - 0x0004
> > > > > > +      - The hardware decoder for this compressed bitstream format (aka coded
> > > > > > +     format) is capable of parsing the bitstream. Applications do not
> > > > > > +     need to parse the bitstream themselves to find the boundaries between
> > > > > > +     frames/fields. This flag can only be used in combination with the
> > > > > > +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> > > > > > +     formats only.
> > > > > 
> > > > > Should this flag be set for stateless codecs as well? It seems a bit over-kill
> > > > > for this case. I am not sure whether "compressed bitstream format" clearly only
> > > > > covers the formats used by stateful decoders and not the ones for stateless
> > > > > decoders.
> > > > 
> > > > I'd suggest using a different name for the flag, because "bitstream
> > > > parser" is actually one of the core differences between stateful and
> > > > stateless. All stateful decoders have bitstream parsers, the only
> > > > difference between the implementations is the unit on which the parser
> > > > operates, i.e. full stream, frame, NALU.
> > > > 
> > > > Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
> > > > framed/sliced chunks)?
> > > 
> > > Sure, that seems like a more explicit name regarding what it's supposed to
> > > describe in my opinion.
> 
> I like that name. And this flag is valid for stateful decoders only.

Sorry, I'm not against the name change, but it should be
V4L2_FMT_FLAG_HAS_BYTESTREAM_PARSER (BYTE). Parsers don't support
random bit alignment, so I think usage of bitstream would be miss-
leading. This is playing on words of course, H264 is a bitstream, but
what is passed to the driver is byte aligned, hence a bytestream.

> 
> > > > Regardless of that, it doesn't make sense for a stateless decoder to
> > > > set this flag anyway, because the userspace needs to parse the whole
> > > > stream anyway and the whole stateless API is based on the assumption
> > > > that the userspace splits the bitstream into frames (or slices).
> > > 
> > > Indeed, I agree that it doesn't make sense, but I thought that the name of the
> > > flag could be confusing. Since there is no direct equivalency between
> > > "stateless" and "doesn't parse the bitstream" (as we've seen with the rockchip
> > > decoder needing to parse the slice header on its own), that could have been
> > > ambiguous. I think the name you're suggesting mostly solves this concern.
> > > 
> > > I'm still a bit unsure about what "compressed formats" entails or not, so it
> > > could be good to explicitly mention that this applies to stateful decoders only
> > > (but it's just a suggestion, advanced users of the API will probably find it
> > > straightforward).
> > 
> > My understanding is that a compressed format is any format that
> > doesn't have a directly accessible 2D pixel matrix in its memory
> > representation, so all the bitstream formats should have it set.
> 
> Correct.
> 
> Regards,
> 
> 	Hans
> 
> > Best regards,
> > Tomasz
> > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Hans Verkuil]

If a driver sets a FMT flag in the enum_fmt op, then that will be
ignored since v4l_fill_fmtdesc() overwrites it again.

v4l_fill_fmtdesc() should OR its flag, not overwrite it.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 80efc581e3f9..911a20f915c5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
 static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-- 
2.20.1

[PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite

[Hans Verkuil]

If a driver sets a FMT flag in the enum_fmt op, then that will be
ignored since v4l_fill_fmtdesc() overwrites it again.

v4l_fill_fmtdesc() should OR its flag, not overwrite it.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 80efc581e3f9..911a20f915c5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strscpy(fmt->description, descr, sz) < 0);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
 static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-- 
2.20.1