RK3288 VP8 decoding support

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.

---
 Documentation/media/uapi/v4l/biblio.rst       |  10 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 323 ++++++++++++++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |  20 ++
 drivers/media/v4l2-core/v4l2-ctrls.c          |   8 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
 include/media/v4l2-ctrls.h                    |   3 +
 include/media/vp8-ctrls.h                     | 110 ++++++
 7 files changed, 475 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h

Documentation/media/uapi/v4l/biblio.rst +10 -0

index 8f4eb8823d82..ad2ff258afa8 100644
@@ -395,3 +395,13 @@ colimg
 :title:     Color Imaging: Fundamentals and Applications
 
 :author:    Erik Reinhard et al.
+
+.. _vp8:
+
+VP8
+===
+
+
+:title:     RFC 6386: "VP8 Data Format and Decoding Guide"
+
+:author:    J. Bankoski et al.

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +323 -0

index d6ea2ffd65c5..aef335f175cd 100644
@@ -2234,6 +2234,329 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Quantization parameter for a P frame for FWHT. Valid range: from 1
     to 31.
 
+.. _v4l2-mpeg-vp8:
+
+``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (struct)``
+    Specifies the frame parameters for the associated VP8 parsed frame data.
+    This includes the necessary parameters for
+    configuring a stateless hardware decoding pipeline for VP8.
+    The bitstream parameters are defined according to :ref:`vp8`.
+
+    .. note::
+
+       This compound control is not yet part of the public kernel API and
+       it is expected to change.
+
+.. c:type:: v4l2_ctrl_vp8_frame_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
+
+.. flat-table:: struct v4l2_ctrl_vp8_frame_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - struct :c:type:`v4l2_vp8_segment_header`
+      - ``segment_header``
+      - Structure with segment-based adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_loopfilter_header`
+      - ``loopfilter_header``
+      - Structure with loop filter level adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_quantization_header`
+      - ``quant_header``
+      - Structure with VP8 dequantization indices metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_header`
+      - ``entropy_header``
+      - Structure with VP8 entropy coder probabilities metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_coder_state`
+      - ``coder_state``
+      - Structure with VP8 entropy coder state.
+    * - __u16
+      - ``width``
+      - The width of the frame. Must be set for all frames.
+    * - __u16
+      - ``height``
+      - The height of the frame. Must be set for all frames.
+    * - __u8
+      - ``horizontal_scale``
+      - Horizontal scaling factor.
+    * - __u8
+      - ``vertical_scaling factor``
+      - Vertical scale.
+    * - __u8
+      - ``version``
+      - Bitstream version.
+    * - __u8
+      - ``prob_skip_false``
+      - Indicates the probability that the macroblock is not skipped.
+    * - __u8
+      - ``prob_intra``
+      - Indicates the probability that a macroblock is intra-predicted.
+    * - __u8
+      - ``prob_last``
+      - Indicates the probability that the last reference frame is used
+        for inter-prediction
+    * - __u8
+      - ``prob_gf``
+      - Indicates the probability that the golden reference frame is used
+        for inter-prediction
+    * - __u8
+      - ``num_dct_parts``
+      - Number of DCT coefficients partitions.
+    * - __u32
+      - ``first_part_size``
+      - Size of the first partition, i.e. the control partition.
+    * - __u32
+      - ``macroblock_bit_offset``
+      - Offset in bits of macroblock data in the first partition. NOT IN THE SPEC!
+    * - __u32
+      - ``dct_part_sizes[8]``
+      - DCT coefficients sizes.
+    * - __u64
+      - ``last_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``golden_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``alt_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as alternate reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``flags``
+      - See :ref:`Frame Header Flags <vp8_frame_header_flags>`
+
+.. _vp8_frame_header_flags:
+
+``Frame Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME``
+      - 0x01
+      - Inidicates if the frame is a key frame.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL``
+      - 0x02
+      - Experimental bitstream.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME``
+      - 0x04
+      - Show frame flag, indicates if the frame is for display.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF``
+      - 0x08
+      - Enable/disable skipping of macroblocks with no non-zero coefficients.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN``
+      - 0x10
+      - Sign of motion vectors when the golden frame is referenced.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT``
+      - 0x20
+      - Sign of motion vectors when the alt frame is referenced.
+
+.. c:type:: v4l2_vp8_entropy_coder_state
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_coder_state
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``range``
+      -
+    * - __u8
+      - ``value``
+      -
+    * - __u8
+      - ``bit_count``
+      -
+    * - __u8
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_segment_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_segment_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``quant_update[4]``
+      - Signed quantizer value update.
+    * - __s8
+      - ``lf_update[4]``
+      - Signed loop filter level value update.
+    * - __u8
+      - ``segment_probs[3]``
+      - Segment probabilities.
+    * - __u8
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Segment Header Flags <vp8_segment_header_flags>`
+
+.. _vp8_segment_header_flags:
+
+``Segment Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED``
+      - 0x01
+      - Enable/disable segment-based adjustments.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP``
+      - 0x02
+      - Indicates if the macroblock segmentation map is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA``
+      - 0x04
+      - Indicates if the segment feature data is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE``
+      - 0x08
+      - If is set, the segment feature data mode is delta-value.
+        If cleared, it's absolute-value.
+
+.. c:type:: v4l2_vp8_loopfilter_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_loopfilter_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``ref_frm_delta[4]``
+      - Reference adjustment (signed) delta value.
+    * - __s8
+      - ``mb_mode_delta[4]``
+      - Macroblock prediction mode adjustment (signed) delta value.
+    * - __u8
+      - ``sharpness_level``
+      - Sharpness level
+    * - __u8
+      - ``level``
+      - Filter level
+    * - __u16
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Loopfilter Header Flags <vp8_loopfilter_header_flags>`
+
+.. _vp8_loopfilter_header_flags:
+
+``Loopfilter Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_LF_HEADER_ADJ_ENABLE``
+      - 0x01
+      - Enable/disable macroblock-level loop filter adjustment.
+    * - ``V4L2_VP8_LF_HEADER_DELTA_UPDATE``
+      - 0x02
+      - Indicates if the delta values used in an adjustment are updated.
+    * - ``V4L2_VP8_LF_FILTER_TYPE_SIMPLE``
+      - 0x04
+      - If set, indicates the filter type is simple.
+        If cleared, the filter type is normal.
+
+.. c:type:: v4l2_vp8_quantization_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_quantization_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``y_ac_qi``
+      - Luma AC coefficient table index.
+    * - __s8
+      - ``y_dc_delta``
+      - Luma DC delta vaue.
+    * - __s8
+      - ``y2_dc_delta``
+      - Y2 block DC delta value.
+    * - __s8
+      - ``y2_ac_delta``
+      - Y2 block AC delta value.
+    * - __s8
+      - ``uv_dc_delta``
+      - Chroma DC delta value.
+    * - __s8
+      - ``uv_ac_delta``
+      - Chroma AC delta value.
+    * - __u16
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_entropy_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``coeff_probs[4][8][3][11]``
+      - Coefficient update probabilities.
+    * - __u8
+      - ``y_mode_probs[4]``
+      - Luma mode update probabilities.
+    * - __u8
+      - ``uv_mode_probs[3]``
+      - Chroma mode update probabilities.
+    * - __u8
+      - ``mv_probs[2][19]``
+      - MV decoding update probabilities.
+    * - __u8
+      - ``padding[3]``
+      -
+
 .. raw:: latex
 
     \normalsize

Documentation/media/uapi/v4l/pixfmt-compressed.rst +20 -0

index 4b701fc7653e..f52a7b67023d 100644
@@ -133,6 +133,26 @@ Compressed Formats
       - ``V4L2_PIX_FMT_VP8``
       - 'VP80'
       - VP8 video elementary stream.
+    * .. _V4L2-PIX-FMT-VP8-FRAME:
+
+      - ``V4L2_PIX_FMT_VP8_FRAME``
+      - 'VP8F'
+      - VP8 parsed frame, as extracted from the container.
+	This format is adapted for stateless video decoders that implement a
+	VP8 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`).
+	Metadata associated with the frame to decode is required to be passed
+	through the ``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER`` control.
+	See the :ref:`associated Codec Control IDs <v4l2-mpeg-vp8>`.
+	Exactly one output and one capture buffer must be provided for use with
+	this pixel format. The output buffer must contain the appropriate number
+	of macroblocks to decode a full corresponding frame to the matching
+	capture buffer.
+
+	.. note::
+
+	   This format is not yet part of the public kernel API and it
+	   is expected to change.
+
     * .. _V4L2-PIX-FMT-VP9:
 
       - ``V4L2_PIX_FMT_VP9``

drivers/media/v4l2-core/v4l2-ctrls.c +8 -0

index 371537dd8cd3..7f94c431d94e 100644
@@ -885,6 +885,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:		return "VPX P-Frame QP Value";
 	case V4L2_CID_MPEG_VIDEO_VP8_PROFILE:			return "VP8 Profile";
 	case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:			return "VP9 Profile";
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:		return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";

@@ -1345,6 +1346,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS:
 		*type = V4L2_CTRL_TYPE_H264_DECODE_PARAMS;
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;

@@ -1748,6 +1752,7 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
 	case V4L2_CTRL_TYPE_H264_SLICE_PARAMS:
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
 		return 0;
 
 	default:

@@ -2348,6 +2353,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_params);
 		break;
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);

drivers/media/v4l2-core/v4l2-ioctl.c +1 -0

index b1f4b991dba6..436a13204921 100644
@@ -1331,6 +1331,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_VC1_ANNEX_G:	descr = "VC-1 (SMPTE 412M Annex G)"; break;
 		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
 		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
+		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 FRAME"; break;
 		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
 		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
 		case V4L2_PIX_FMT_FWHT:		descr = "FWHT"; break; /* used in vicodec */

include/media/v4l2-ctrls.h +3 -0

index b4433483af23..6e9dc9c44bb1 100644
@@ -20,6 +20,7 @@
 #include <media/mpeg2-ctrls.h>
 #include <media/fwht-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;

@@ -48,6 +49,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_params:	Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:	Pointer to a struct v4l2_ctrl_h264_decode_params.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {

@@ -65,6 +67,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
 	struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 

include/media/vp8-ctrls.h new file +110 -0

new file mode 100644
index 000000000000..636442dc18aa
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the VP8 state controls for use with stateless VP8
+ * codec drivers.
+ *
+ * It turns out that these structs are not stable yet and will undergo
+ * more changes. So keep them private until they are stable and ready to
+ * become part of the official public API.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F')
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (V4L2_CID_MPEG_BASE + 2000)
+#define V4L2_CTRL_TYPE_VP8_FRAME_HEADER 0x301
+
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA  0x04
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE     0x08
+
+struct v4l2_vp8_segment_header {
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u8 padding;
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HEADER_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HEADER_DELTA_UPDATE	0x02
+#define V4L2_VP8_LF_FILTER_TYPE_SIMPLE	0x04
+struct v4l2_vp8_loopfilter_header {
+	__s8 ref_frm_delta[4];
+	__s8 mb_mode_delta[4];
+	__u8 sharpness_level;
+	__u8 level;
+	__u16 padding;
+	__u32 flags;
+};
+
+struct v4l2_vp8_quantization_header {
+	__u8 y_ac_qi;
+	__s8 y_dc_delta;
+	__s8 y2_dc_delta;
+	__s8 y2_ac_delta;
+	__s8 uv_dc_delta;
+	__s8 uv_ac_delta;
+	__u16 padding;
+};
+
+struct v4l2_vp8_entropy_header {
+	__u8 coeff_probs[4][8][3][11];
+	__u8 y_mode_probs[4];
+	__u8 uv_mode_probs[3];
+	__u8 mv_probs[2][19];
+	__u8 padding[3];
+};
+
+struct v4l2_vp8_entropy_coder_state {
+	__u8 range;
+	__u8 value;
+	__u8 bit_count;
+	__u8 padding;
+};
+
+#define V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME		0x01
+#define V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL		0x02
+#define V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME		0x04
+#define V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF	0x08
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN	0x10
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT	0x20
+
+#define VP8_FRAME_IS_KEY_FRAME(hdr) \
+	(!!((hdr)->flags & V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME))
+
+struct v4l2_ctrl_vp8_frame_header {
+	struct v4l2_vp8_segment_header segment_header;
+	struct v4l2_vp8_loopfilter_header lf_header;
+	struct v4l2_vp8_quantization_header quant_header;
+	struct v4l2_vp8_entropy_header entropy_header;
+	struct v4l2_vp8_entropy_coder_state coder_state;
+
+	__u16 width;
+	__u16 height;
+
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	__u8 version;
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+	__u8 num_dct_parts;
+
+	__u32 first_part_size;
+	__u32 macroblock_bit_offset;
+	__u32 dct_part_sizes[8];
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+
+	__u64 flags;
+};
+
+#endif

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.

---
 Documentation/media/uapi/v4l/biblio.rst       |  10 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 323 ++++++++++++++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |  20 ++
 drivers/media/v4l2-core/v4l2-ctrls.c          |   8 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
 include/media/v4l2-ctrls.h                    |   3 +
 include/media/vp8-ctrls.h                     | 110 ++++++
 7 files changed, 475 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h

Documentation/media/uapi/v4l/biblio.rst +10 -0

index 8f4eb8823d82..ad2ff258afa8 100644
@@ -395,3 +395,13 @@ colimg
 :title:     Color Imaging: Fundamentals and Applications
 
 :author:    Erik Reinhard et al.
+
+.. _vp8:
+
+VP8
+===
+
+
+:title:     RFC 6386: "VP8 Data Format and Decoding Guide"
+
+:author:    J. Bankoski et al.

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +61 -0

index d6ea2ffd65c5..aef335f175cd 100644
@@ -2234,6 +2234,329 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Quantization parameter for a P frame for FWHT. Valid range: from 1
     to 31.
 
+.. _v4l2-mpeg-vp8:
+
+``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (struct)``
+    Specifies the frame parameters for the associated VP8 parsed frame data.
+    This includes the necessary parameters for
+    configuring a stateless hardware decoding pipeline for VP8.
+    The bitstream parameters are defined according to :ref:`vp8`.
+
+    .. note::
+
+       This compound control is not yet part of the public kernel API and
+       it is expected to change.
+
+.. c:type:: v4l2_ctrl_vp8_frame_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
+
+.. flat-table:: struct v4l2_ctrl_vp8_frame_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - struct :c:type:`v4l2_vp8_segment_header`
+      - ``segment_header``
+      - Structure with segment-based adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_loopfilter_header`
+      - ``loopfilter_header``
+      - Structure with loop filter level adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_quantization_header`
+      - ``quant_header``
+      - Structure with VP8 dequantization indices metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_header`
+      - ``entropy_header``
+      - Structure with VP8 entropy coder probabilities metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_coder_state`
+      - ``coder_state``
+      - Structure with VP8 entropy coder state.
+    * - __u16
+      - ``width``
+      - The width of the frame. Must be set for all frames.
+    * - __u16
+      - ``height``
+      - The height of the frame. Must be set for all frames.
+    * - __u8
+      - ``horizontal_scale``
+      - Horizontal scaling factor.
+    * - __u8
+      - ``vertical_scaling factor``
+      - Vertical scale.
+    * - __u8
+      - ``version``
+      - Bitstream version.
+    * - __u8
+      - ``prob_skip_false``
+      - Indicates the probability that the macroblock is not skipped.
+    * - __u8
+      - ``prob_intra``
+      - Indicates the probability that a macroblock is intra-predicted.
+    * - __u8

Quoted 32 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.

---
 Documentation/media/uapi/v4l/biblio.rst       |  10 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 323 ++++++++++++++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |  20 ++
 drivers/media/v4l2-core/v4l2-ctrls.c          |   8 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
 include/media/v4l2-ctrls.h                    |   3 +
 include/media/vp8-ctrls.h                     | 110 ++++++
 7 files changed, 475 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h

Documentation/media/uapi/v4l/ext-ctrls-codec.rst enum v4l2_mpeg_video_h264_hierarchical_coding_type - · line 2307

Quoted 3 lines

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +3 -0

+    * - __u8
+      - ``num_dct_parts``
+      - Number of DCT coefficients partitions.

I assume this must be no larger than 8. Is this mandated by the spec? If so, should it be documented here and checked by v4l2-core?

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.

---
 Documentation/media/uapi/v4l/biblio.rst       |  10 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 323 ++++++++++++++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |  20 ++
 drivers/media/v4l2-core/v4l2-ctrls.c          |   8 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
 include/media/v4l2-ctrls.h                    |   3 +
 include/media/vp8-ctrls.h                     | 110 ++++++
 7 files changed, 475 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h

Documentation/media/uapi/v4l/biblio.rst +10 -0

index 8f4eb8823d82..ad2ff258afa8 100644
@@ -395,3 +395,13 @@ colimg
 :title:     Color Imaging: Fundamentals and Applications
 
 :author:    Erik Reinhard et al.
+
+.. _vp8:
+
+VP8
+===
+
+
+:title:     RFC 6386: "VP8 Data Format and Decoding Guide"
+
+:author:    J. Bankoski et al.

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +2 -0

index d6ea2ffd65c5..aef335f175cd 100644
@@ -2234,6 +2234,329 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Quantization parameter for a P frame for FWHT. Valid range: from 1
     to 31.
 
+.. _v4l2-mpeg-vp8:
+

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +55 -0

+
+.. c:type:: v4l2_ctrl_vp8_frame_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
+
+.. flat-table:: struct v4l2_ctrl_vp8_frame_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - struct :c:type:`v4l2_vp8_segment_header`
+      - ``segment_header``
+      - Structure with segment-based adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_loopfilter_header`
+      - ``loopfilter_header``
+      - Structure with loop filter level adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_quantization_header`
+      - ``quant_header``
+      - Structure with VP8 dequantization indices metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_header`
+      - ``entropy_header``
+      - Structure with VP8 entropy coder probabilities metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_coder_state`
+      - ``coder_state``
+      - Structure with VP8 entropy coder state.
+    * - __u16
+      - ``width``
+      - The width of the frame. Must be set for all frames.
+    * - __u16
+      - ``height``
+      - The height of the frame. Must be set for all frames.
+    * - __u8
+      - ``horizontal_scale``
+      - Horizontal scaling factor.
+    * - __u8
+      - ``vertical_scaling factor``
+      - Vertical scale.
+    * - __u8
+      - ``version``
+      - Bitstream version.
+    * - __u8
+      - ``prob_skip_false``
+      - Indicates the probability that the macroblock is not skipped.
+    * - __u8
+      - ``prob_intra``
+      - Indicates the probability that a macroblock is intra-predicted.
+    * - __u8
+      - ``prob_last``
+      - Indicates the probability that the last reference frame is used
+        for inter-prediction
+    * - __u8
+      - ``prob_gf``
+      - Indicates the probability that the golden reference frame is used

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +246 -0

+    * - __u32
+      - ``dct_part_sizes[8]``
+      - DCT coefficients sizes.
+    * - __u64
+      - ``last_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``golden_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``alt_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as alternate reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``flags``
+      - See :ref:`Frame Header Flags <vp8_frame_header_flags>`
+
+.. _vp8_frame_header_flags:
+
+``Frame Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME``
+      - 0x01
+      - Inidicates if the frame is a key frame.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL``
+      - 0x02
+      - Experimental bitstream.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME``
+      - 0x04
+      - Show frame flag, indicates if the frame is for display.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF``
+      - 0x08
+      - Enable/disable skipping of macroblocks with no non-zero coefficients.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN``
+      - 0x10
+      - Sign of motion vectors when the golden frame is referenced.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT``
+      - 0x20
+      - Sign of motion vectors when the alt frame is referenced.
+
+.. c:type:: v4l2_vp8_entropy_coder_state
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_coder_state
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``range``
+      -
+    * - __u8
+      - ``value``
+      -
+    * - __u8
+      - ``bit_count``
+      -
+    * - __u8
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_segment_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_segment_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``quant_update[4]``
+      - Signed quantizer value update.
+    * - __s8
+      - ``lf_update[4]``
+      - Signed loop filter level value update.
+    * - __u8
+      - ``segment_probs[3]``
+      - Segment probabilities.
+    * - __u8
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Segment Header Flags <vp8_segment_header_flags>`
+
+.. _vp8_segment_header_flags:
+
+``Segment Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED``
+      - 0x01
+      - Enable/disable segment-based adjustments.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP``
+      - 0x02
+      - Indicates if the macroblock segmentation map is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA``
+      - 0x04
+      - Indicates if the segment feature data is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE``
+      - 0x08
+      - If is set, the segment feature data mode is delta-value.
+        If cleared, it's absolute-value.
+
+.. c:type:: v4l2_vp8_loopfilter_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_loopfilter_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``ref_frm_delta[4]``
+      - Reference adjustment (signed) delta value.
+    * - __s8
+      - ``mb_mode_delta[4]``
+      - Macroblock prediction mode adjustment (signed) delta value.
+    * - __u8
+      - ``sharpness_level``
+      - Sharpness level
+    * - __u8
+      - ``level``
+      - Filter level
+    * - __u16
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Loopfilter Header Flags <vp8_loopfilter_header_flags>`
+
+.. _vp8_loopfilter_header_flags:
+
+``Loopfilter Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_LF_HEADER_ADJ_ENABLE``
+      - 0x01
+      - Enable/disable macroblock-level loop filter adjustment.
+    * - ``V4L2_VP8_LF_HEADER_DELTA_UPDATE``
+      - 0x02
+      - Indicates if the delta values used in an adjustment are updated.
+    * - ``V4L2_VP8_LF_FILTER_TYPE_SIMPLE``
+      - 0x04
+      - If set, indicates the filter type is simple.
+        If cleared, the filter type is normal.
+
+.. c:type:: v4l2_vp8_quantization_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_quantization_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``y_ac_qi``
+      - Luma AC coefficient table index.
+    * - __s8
+      - ``y_dc_delta``
+      - Luma DC delta vaue.
+    * - __s8
+      - ``y2_dc_delta``
+      - Y2 block DC delta value.
+    * - __s8
+      - ``y2_ac_delta``
+      - Y2 block AC delta value.
+    * - __s8
+      - ``uv_dc_delta``
+      - Chroma DC delta value.
+    * - __s8
+      - ``uv_ac_delta``
+      - Chroma AC delta value.
+    * - __u16
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_entropy_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``coeff_probs[4][8][3][11]``
+      - Coefficient update probabilities.
+    * - __u8
+      - ``y_mode_probs[4]``
+      - Luma mode update probabilities.
+    * - __u8
+      - ``uv_mode_probs[3]``
+      - Chroma mode update probabilities.
+    * - __u8
+      - ``mv_probs[2][19]``
+      - MV decoding update probabilities.
+    * - __u8
+      - ``padding[3]``
+      -
+
 .. raw:: latex
 
     \normalsize

Documentation/media/uapi/v4l/pixfmt-compressed.rst +8 -0

index 4b701fc7653e..f52a7b67023d 100644
@@ -133,6 +133,26 @@ Compressed Formats
       - ``V4L2_PIX_FMT_VP8``
       - 'VP80'
       - VP8 video elementary stream.
+    * .. _V4L2-PIX-FMT-VP8-FRAME:
+
+      - ``V4L2_PIX_FMT_VP8_FRAME``
+      - 'VP8F'
+      - VP8 parsed frame, as extracted from the container.
+	This format is adapted for stateless video decoders that implement a
+	VP8 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`).
+	Metadata associated with the frame to decode is required to be passed

Documentation/media/uapi/v4l/pixfmt-compressed.rst +2 -0

+	   is expected to change.
+
     * .. _V4L2-PIX-FMT-VP9:
 
       - ``V4L2_PIX_FMT_VP9``

drivers/media/v4l2-core/v4l2-ctrls.c +8 -0

index 371537dd8cd3..7f94c431d94e 100644
@@ -885,6 +885,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:		return "VPX P-Frame QP Value";
 	case V4L2_CID_MPEG_VIDEO_VP8_PROFILE:			return "VP8 Profile";
 	case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:			return "VP9 Profile";
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:		return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";

@@ -1345,6 +1346,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS:
 		*type = V4L2_CTRL_TYPE_H264_DECODE_PARAMS;
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;

@@ -1748,6 +1752,7 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
 	case V4L2_CTRL_TYPE_H264_SLICE_PARAMS:
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
 		return 0;
 
 	default:

@@ -2348,6 +2353,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_params);
 		break;
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);

drivers/media/v4l2-core/v4l2-ioctl.c +1 -0

index b1f4b991dba6..436a13204921 100644
@@ -1331,6 +1331,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_VC1_ANNEX_G:	descr = "VC-1 (SMPTE 412M Annex G)"; break;
 		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
 		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
+		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 FRAME"; break;
 		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
 		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
 		case V4L2_PIX_FMT_FWHT:		descr = "FWHT"; break; /* used in vicodec */

include/media/v4l2-ctrls.h +3 -0

index b4433483af23..6e9dc9c44bb1 100644
@@ -20,6 +20,7 @@
 #include <media/mpeg2-ctrls.h>
 #include <media/fwht-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;

@@ -48,6 +49,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_params:	Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:	Pointer to a struct v4l2_ctrl_h264_decode_params.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {

@@ -65,6 +67,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
 	struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 

include/media/vp8-ctrls.h new file +100 -0

new file mode 100644
index 000000000000..636442dc18aa
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the VP8 state controls for use with stateless VP8
+ * codec drivers.
+ *
+ * It turns out that these structs are not stable yet and will undergo
+ * more changes. So keep them private until they are stable and ready to
+ * become part of the official public API.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F')
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (V4L2_CID_MPEG_BASE + 2000)
+#define V4L2_CTRL_TYPE_VP8_FRAME_HEADER 0x301
+
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA  0x04
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE     0x08
+
+struct v4l2_vp8_segment_header {
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u8 padding;
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HEADER_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HEADER_DELTA_UPDATE	0x02
+#define V4L2_VP8_LF_FILTER_TYPE_SIMPLE	0x04
+struct v4l2_vp8_loopfilter_header {
+	__s8 ref_frm_delta[4];
+	__s8 mb_mode_delta[4];
+	__u8 sharpness_level;
+	__u8 level;
+	__u16 padding;
+	__u32 flags;
+};
+
+struct v4l2_vp8_quantization_header {
+	__u8 y_ac_qi;
+	__s8 y_dc_delta;
+	__s8 y2_dc_delta;
+	__s8 y2_ac_delta;
+	__s8 uv_dc_delta;
+	__s8 uv_ac_delta;
+	__u16 padding;
+};
+
+struct v4l2_vp8_entropy_header {
+	__u8 coeff_probs[4][8][3][11];
+	__u8 y_mode_probs[4];
+	__u8 uv_mode_probs[3];
+	__u8 mv_probs[2][19];
+	__u8 padding[3];
+};
+
+struct v4l2_vp8_entropy_coder_state {
+	__u8 range;
+	__u8 value;
+	__u8 bit_count;
+	__u8 padding;
+};
+
+#define V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME		0x01
+#define V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL		0x02
+#define V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME		0x04
+#define V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF	0x08
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN	0x10
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT	0x20
+
+#define VP8_FRAME_IS_KEY_FRAME(hdr) \
+	(!!((hdr)->flags & V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME))
+
+struct v4l2_ctrl_vp8_frame_header {
+	struct v4l2_vp8_segment_header segment_header;
+	struct v4l2_vp8_loopfilter_header lf_header;
+	struct v4l2_vp8_quantization_header quant_header;
+	struct v4l2_vp8_entropy_header entropy_header;
+	struct v4l2_vp8_entropy_coder_state coder_state;
+
+	__u16 width;
+	__u16 height;
+
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	__u8 version;
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+	__u8 num_dct_parts;
+
+	__u32 first_part_size;
+	__u32 macroblock_bit_offset;

Quoted 58 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

Add the parsed VP8 frame pixel format and controls, to be used with the new stateless decoder API for VP8 to provide parameters for accelerator (aka stateless) codecs.

Signed-off-by Pawel Osciak <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

--
Changes from v1:
* Move 1-bit fields to flags in the respective structures.
* Add padding fields to make all structures 8-byte aligned.
* Reorder fields where needed to avoid padding as much as possible.
* Fix documentation as needed.

Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.

---
 Documentation/media/uapi/v4l/biblio.rst       |  10 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 323 ++++++++++++++++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |  20 ++
 drivers/media/v4l2-core/v4l2-ctrls.c          |   8 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
 include/media/v4l2-ctrls.h                    |   3 +
 include/media/vp8-ctrls.h                     | 110 ++++++
 7 files changed, 475 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h

Documentation/media/uapi/v4l/biblio.rst +10 -0

index 8f4eb8823d82..ad2ff258afa8 100644
@@ -395,3 +395,13 @@ colimg
 :title:     Color Imaging: Fundamentals and Applications
 
 :author:    Erik Reinhard et al.
+
+.. _vp8:
+
+VP8
+===
+
+
+:title:     RFC 6386: "VP8 Data Format and Decoding Guide"
+
+:author:    J. Bankoski et al.

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +0 -0

index d6ea2ffd65c5..aef335f175cd 100644
@@ -2234,6 +2234,329 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Quantization parameter for a P frame for FWHT. Valid range: from 1
     to 31.
 

Documentation/media/uapi/v4l/ext-ctrls-codec.rst enum v4l2_mpeg_video_h264_hierarchical_coding_type - · line 2248

Quoted 12 lines

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +12 -0

+.. _v4l2-mpeg-vp8:
+
+``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (struct)``
+    Specifies the frame parameters for the associated VP8 parsed frame data.
+    This includes the necessary parameters for
+    configuring a stateless hardware decoding pipeline for VP8.
+    The bitstream parameters are defined according to :ref:`vp8`.
+
+    .. note::
+
+       This compound control is not yet part of the public kernel API and
+       it is expected to change.

nit: I'd remove the "it"

Quoted 60 lines

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +60 -0

+
+.. c:type:: v4l2_ctrl_vp8_frame_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
+
+.. flat-table:: struct v4l2_ctrl_vp8_frame_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - struct :c:type:`v4l2_vp8_segment_header`
+      - ``segment_header``
+      - Structure with segment-based adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_loopfilter_header`
+      - ``loopfilter_header``
+      - Structure with loop filter level adjustments metadata.
+    * - struct :c:type:`v4l2_vp8_quantization_header`
+      - ``quant_header``
+      - Structure with VP8 dequantization indices metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_header`
+      - ``entropy_header``
+      - Structure with VP8 entropy coder probabilities metadata.
+    * - struct :c:type:`v4l2_vp8_entropy_coder_state`
+      - ``coder_state``
+      - Structure with VP8 entropy coder state.
+    * - __u16
+      - ``width``
+      - The width of the frame. Must be set for all frames.
+    * - __u16
+      - ``height``
+      - The height of the frame. Must be set for all frames.
+    * - __u8
+      - ``horizontal_scale``
+      - Horizontal scaling factor.
+    * - __u8
+      - ``vertical_scaling factor``
+      - Vertical scale.
+    * - __u8
+      - ``version``
+      - Bitstream version.
+    * - __u8
+      - ``prob_skip_false``
+      - Indicates the probability that the macroblock is not skipped.
+    * - __u8
+      - ``prob_intra``
+      - Indicates the probability that a macroblock is intra-predicted.
+    * - __u8
+      - ``prob_last``
+      - Indicates the probability that the last reference frame is used
+        for inter-prediction
+    * - __u8
+      - ``prob_gf``
+      - Indicates the probability that the golden reference frame is used
+        for inter-prediction
+    * - __u8
+      - ``num_dct_parts``
+      - Number of DCT coefficients partitions.
+    * - __u32

Documentation/media/uapi/v4l/ext-ctrls-codec.rst enum v4l2_mpeg_video_h264_hierarchical_coding_type - · line 2313

Quoted 5 lines

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +5 -0

+      - ``first_part_size``
+      - Size of the first partition, i.e. the control partition.
+    * - __u32
+      - ``macroblock_bit_offset``
+      - Offset in bits of macroblock data in the first partition. NOT IN THE SPEC!

nit: "NOT IN THE SPEC" -> "This value is not explicit in the frame header."

I am right to think that this is basically the size in bits of the frame header ? Maybe it could be another way to formulate it ? I'm just trying to think of formulation that will better guide the developers implementing the parser feeding this. You basically need to parse the header to get this size (as everything is dynamically sized).

Quoted 269 lines

Documentation/media/uapi/v4l/ext-ctrls-codec.rst +246 -0

+    * - __u32
+      - ``dct_part_sizes[8]``
+      - DCT coefficients sizes.
+    * - __u64
+      - ``last_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``golden_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``alt_frame_ts``
+      - Timestamp for the V4L2 capture buffer to use as alternate reference frame, used
+        with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+	struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+	function to convert the struct :c:type:`timeval` in struct
+	:c:type:`v4l2_buffer` to a __u64.
+    * - __u64
+      - ``flags``
+      - See :ref:`Frame Header Flags <vp8_frame_header_flags>`
+
+.. _vp8_frame_header_flags:
+
+``Frame Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME``
+      - 0x01
+      - Inidicates if the frame is a key frame.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL``
+      - 0x02
+      - Experimental bitstream.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME``
+      - 0x04
+      - Show frame flag, indicates if the frame is for display.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF``
+      - 0x08
+      - Enable/disable skipping of macroblocks with no non-zero coefficients.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN``
+      - 0x10
+      - Sign of motion vectors when the golden frame is referenced.
+    * - ``V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT``
+      - 0x20
+      - Sign of motion vectors when the alt frame is referenced.
+
+.. c:type:: v4l2_vp8_entropy_coder_state
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_coder_state
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``range``
+      -
+    * - __u8
+      - ``value``
+      -
+    * - __u8
+      - ``bit_count``
+      -
+    * - __u8
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_segment_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_segment_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``quant_update[4]``
+      - Signed quantizer value update.
+    * - __s8
+      - ``lf_update[4]``
+      - Signed loop filter level value update.
+    * - __u8
+      - ``segment_probs[3]``
+      - Segment probabilities.
+    * - __u8
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Segment Header Flags <vp8_segment_header_flags>`
+
+.. _vp8_segment_header_flags:
+
+``Segment Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED``
+      - 0x01
+      - Enable/disable segment-based adjustments.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP``
+      - 0x02
+      - Indicates if the macroblock segmentation map is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA``
+      - 0x04
+      - Indicates if the segment feature data is updated in this frame.
+    * - ``V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE``
+      - 0x08
+      - If is set, the segment feature data mode is delta-value.
+        If cleared, it's absolute-value.
+
+.. c:type:: v4l2_vp8_loopfilter_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_loopfilter_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __s8
+      - ``ref_frm_delta[4]``
+      - Reference adjustment (signed) delta value.
+    * - __s8
+      - ``mb_mode_delta[4]``
+      - Macroblock prediction mode adjustment (signed) delta value.
+    * - __u8
+      - ``sharpness_level``
+      - Sharpness level
+    * - __u8
+      - ``level``
+      - Filter level
+    * - __u16
+      - ``padding``
+      -
+    * - __u32
+      - ``flags``
+      - See :ref:`Loopfilter Header Flags <vp8_loopfilter_header_flags>`
+
+.. _vp8_loopfilter_header_flags:
+
+``Loopfilter Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - ``V4L2_VP8_LF_HEADER_ADJ_ENABLE``
+      - 0x01
+      - Enable/disable macroblock-level loop filter adjustment.
+    * - ``V4L2_VP8_LF_HEADER_DELTA_UPDATE``
+      - 0x02
+      - Indicates if the delta values used in an adjustment are updated.
+    * - ``V4L2_VP8_LF_FILTER_TYPE_SIMPLE``
+      - 0x04
+      - If set, indicates the filter type is simple.
+        If cleared, the filter type is normal.
+
+.. c:type:: v4l2_vp8_quantization_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_quantization_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``y_ac_qi``
+      - Luma AC coefficient table index.
+    * - __s8
+      - ``y_dc_delta``
+      - Luma DC delta vaue.
+    * - __s8
+      - ``y2_dc_delta``
+      - Y2 block DC delta value.
+    * - __s8
+      - ``y2_ac_delta``
+      - Y2 block AC delta value.
+    * - __s8
+      - ``uv_dc_delta``
+      - Chroma DC delta value.
+    * - __s8
+      - ``uv_ac_delta``
+      - Chroma AC delta value.
+    * - __u16
+      - ``padding``
+      -
+
+.. c:type:: v4l2_vp8_entropy_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_header
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``coeff_probs[4][8][3][11]``
+      - Coefficient update probabilities.
+    * - __u8
+      - ``y_mode_probs[4]``
+      - Luma mode update probabilities.
+    * - __u8
+      - ``uv_mode_probs[3]``
+      - Chroma mode update probabilities.
+    * - __u8
+      - ``mv_probs[2][19]``
+      - MV decoding update probabilities.
+    * - __u8
+      - ``padding[3]``
+      -
+
 .. raw:: latex
 
     \normalsize

Documentation/media/uapi/v4l/pixfmt-compressed.rst +12 -0

index 4b701fc7653e..f52a7b67023d 100644
@@ -133,6 +133,26 @@ Compressed Formats
       - ``V4L2_PIX_FMT_VP8``
       - 'VP80'
       - VP8 video elementary stream.
+    * .. _V4L2-PIX-FMT-VP8-FRAME:
+
+      - ``V4L2_PIX_FMT_VP8_FRAME``
+      - 'VP8F'
+      - VP8 parsed frame, as extracted from the container.
+	This format is adapted for stateless video decoders that implement a
+	VP8 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`).
+	Metadata associated with the frame to decode is required to be passed
+	through the ``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER`` control.
+	See the :ref:`associated Codec Control IDs <v4l2-mpeg-vp8>`.
+	Exactly one output and one capture buffer must be provided for use with
+	this pixel format. The output buffer must contain the appropriate number

Documentation/media/uapi/v4l/pixfmt-compressed.rst Compressed Formats · line 153

Quoted 6 lines

Documentation/media/uapi/v4l/pixfmt-compressed.rst +6 -0

+	of macroblocks to decode a full corresponding frame to the matching
+	capture buffer.
+
+	.. note::
+
+	   This format is not yet part of the public kernel API and it

nit: Same, would drop the "it".

Quoted 197 lines

Documentation/media/uapi/v4l/pixfmt-compressed.rst +2 -0

+	   is expected to change.
+
     * .. _V4L2-PIX-FMT-VP9:
 
       - ``V4L2_PIX_FMT_VP9``

drivers/media/v4l2-core/v4l2-ctrls.c +8 -0

index 371537dd8cd3..7f94c431d94e 100644
@@ -885,6 +885,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:		return "VPX P-Frame QP Value";
 	case V4L2_CID_MPEG_VIDEO_VP8_PROFILE:			return "VP8 Profile";
 	case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:			return "VP9 Profile";
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:		return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";

@@ -1345,6 +1346,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS:
 		*type = V4L2_CTRL_TYPE_H264_DECODE_PARAMS;
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;

@@ -1748,6 +1752,7 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
 	case V4L2_CTRL_TYPE_H264_SLICE_PARAMS:
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
 		return 0;
 
 	default:

@@ -2348,6 +2353,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_params);
 		break;
+	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);

drivers/media/v4l2-core/v4l2-ioctl.c +1 -0

index b1f4b991dba6..436a13204921 100644
@@ -1331,6 +1331,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_VC1_ANNEX_G:	descr = "VC-1 (SMPTE 412M Annex G)"; break;
 		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
 		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
+		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 FRAME"; break;
 		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
 		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
 		case V4L2_PIX_FMT_FWHT:		descr = "FWHT"; break; /* used in vicodec */

include/media/v4l2-ctrls.h +3 -0

index b4433483af23..6e9dc9c44bb1 100644
@@ -20,6 +20,7 @@
 #include <media/mpeg2-ctrls.h>
 #include <media/fwht-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;

@@ -48,6 +49,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_params:	Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:	Pointer to a struct v4l2_ctrl_h264_decode_params.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {

@@ -65,6 +67,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
 	struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 

include/media/vp8-ctrls.h new file +106 -0

new file mode 100644
index 000000000000..636442dc18aa
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the VP8 state controls for use with stateless VP8
+ * codec drivers.
+ *
+ * It turns out that these structs are not stable yet and will undergo
+ * more changes. So keep them private until they are stable and ready to
+ * become part of the official public API.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F')
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER (V4L2_CID_MPEG_BASE + 2000)
+#define V4L2_CTRL_TYPE_VP8_FRAME_HEADER 0x301
+
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_UPDATE_FEATURE_DATA  0x04
+#define V4L2_VP8_SEGMENT_HEADER_FLAG_DELTA_VALUE_MODE     0x08
+
+struct v4l2_vp8_segment_header {
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u8 padding;
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HEADER_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HEADER_DELTA_UPDATE	0x02
+#define V4L2_VP8_LF_FILTER_TYPE_SIMPLE	0x04
+struct v4l2_vp8_loopfilter_header {
+	__s8 ref_frm_delta[4];
+	__s8 mb_mode_delta[4];
+	__u8 sharpness_level;
+	__u8 level;
+	__u16 padding;
+	__u32 flags;
+};
+
+struct v4l2_vp8_quantization_header {
+	__u8 y_ac_qi;
+	__s8 y_dc_delta;
+	__s8 y2_dc_delta;
+	__s8 y2_ac_delta;
+	__s8 uv_dc_delta;
+	__s8 uv_ac_delta;
+	__u16 padding;
+};
+
+struct v4l2_vp8_entropy_header {
+	__u8 coeff_probs[4][8][3][11];
+	__u8 y_mode_probs[4];
+	__u8 uv_mode_probs[3];
+	__u8 mv_probs[2][19];
+	__u8 padding[3];
+};
+
+struct v4l2_vp8_entropy_coder_state {
+	__u8 range;
+	__u8 value;
+	__u8 bit_count;
+	__u8 padding;
+};
+
+#define V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME		0x01
+#define V4L2_VP8_FRAME_HEADER_FLAG_EXPERIMENTAL		0x02
+#define V4L2_VP8_FRAME_HEADER_FLAG_SHOW_FRAME		0x04
+#define V4L2_VP8_FRAME_HEADER_FLAG_MB_NO_SKIP_COEFF	0x08
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_GOLDEN	0x10
+#define V4L2_VP8_FRAME_HEADER_FLAG_SIGN_BIAS_ALT	0x20
+
+#define VP8_FRAME_IS_KEY_FRAME(hdr) \
+	(!!((hdr)->flags & V4L2_VP8_FRAME_HEADER_FLAG_KEY_FRAME))
+
+struct v4l2_ctrl_vp8_frame_header {
+	struct v4l2_vp8_segment_header segment_header;
+	struct v4l2_vp8_loopfilter_header lf_header;
+	struct v4l2_vp8_quantization_header quant_header;
+	struct v4l2_vp8_entropy_header entropy_header;
+	struct v4l2_vp8_entropy_coder_state coder_state;
+
+	__u16 width;
+	__u16 height;
+
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	__u8 version;
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+	__u8 num_dct_parts;
+
+	__u32 first_part_size;
+	__u32 macroblock_bit_offset;
+	__u32 dct_part_sizes[8];
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+

include/media/vp8-ctrls.h line 110

Quoted 4 lines

include/media/vp8-ctrls.h new file +4 -0

+	__u64 flags;
+};
+
+#endif

I don't think any of my comments strictly requires a v3, could be later enhancements. You told me you wrote and ran a test for 32/64 compatibility, so I on did a light visual check.

(even if you already got that from Tomasz)

Reviewed-by Nicolas Dufresne <[email protected]>

Oops, this is just an internal note, it seems I forgot to remove this one.

Quoted 6 lines

I am right to think that this is basically the size in bits of the frame header ? Maybe it could be another way to formulate it ? I'm just trying to think of formulation that will better guide the developers implementing the parser feeding this. You basically need to parse the header to get this size (as everything is dynamically sized).

Depending what you call "frame header", then yes, it's the size in bits.

                             first_part_size          parttion_sizes[0]
                                ^                     ^
                                |                     |
                       +--------+------+        +-----+-----+
                       | control part  |        |           |
   +--------+----------------+------------------+-----------+-----+-----------+
   | tag 3B | extra 7B | hdr | mb_data | dct sz | dct part0 | ... | dct partn |
   +--------+-----------------------------------+-----------+-----+-----------+

The above shows a VP8 frame, "macroblock_bit_offset" is the size in bits of the "hdr" portion: i.e. the header of the first partition (aka control partition).

Thinking about it, the current description is quite confusing.

How about:

"Size in bits of the frame header. In other words, this the size in bits of the header portion of the first partition".

drivers/staging/media/hantro/rk3288_vpu_hw.c +1 -0

index bcacc4f51093..470e803e25a6 100644
@@ -74,6 +74,19 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
 			.step_height = MPEG2_MB_DIM,
 		},
 	},
+	{

drivers/staging/media/hantro/rk3288_vpu_hw.c +2 -0

+		},
+	},
 };

From: ZhiChao Yu <[email protected]>

Introduce VP8 decoding support in RK3288.

Signed-off-by ZhiChao Yu <[email protected]>

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
Changes from v1:
* Place operators at the end of each line.
* Update to uAPI changes.

---
 drivers/staging/media/hantro/Makefile         |   4 +-
 drivers/staging/media/hantro/hantro.h         |   5 +
 drivers/staging/media/hantro/hantro_drv.c     |   6 +
 .../staging/media/hantro/hantro_g1_vp8_dec.c  | 552 ++++++++++++++++++
 drivers/staging/media/hantro/hantro_hw.h      |  17 +
 drivers/staging/media/hantro/hantro_v4l2.c    |   1 +
 drivers/staging/media/hantro/hantro_vp8.c     | 188 ++++++
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |  22 +-
 8 files changed, 793 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_vp8.c

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +0 -0

new file mode 100644
index 000000000000..31d31faae4aa
@@ -0,0 +1,552 @@

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition base address regs */
+static const struct vp8_dec_reg vp8_dec_dct_base[8] = {

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +3 -0

+static void cfg_parts(struct hantro_ctx *ctx,
+		      const struct v4l2_ctrl_vp8_frame_header *hdr)
+{

Hi Ezequiel

Quoted 8 lines · Ezequiel Garcia

From: ZhiChao Yu <[email protected]>

Introduce VP8 decoding support in RK3288.

Signed-off-by ZhiChao Yu <[email protected]>

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

I have just tried this (with broken userspace) and got a crash in cfg_parts, see below for details:

[  114.308757] Unable to handle kernel paging request at virtual address ffff0000112b0002
[  114.316691] Mem abort info:
[  114.319503]   ESR = 0x96000021
[  114.322576]   Exception class = DABT (current EL), IL = 32 bits
[  114.328513]   SET = 0, FnV = 0
[  114.331586]   EA = 0, S1PTW = 0
[  114.334744] Data abort info:
[  114.337626]   ISV = 0, ISS = 0x00000021
[  114.341479]   CM = 0, WnR = 0
[  114.344466] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000040d61000
[  114.351185] [ffff0000112b0002] pgd=00000000dffff003, pud=00000000dfffe003, pmd=00000000dbf36003, pte=00e8000038300707 [  114.361822] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[  114.367394] Modules linked in: crct10dif_ce hantro_vpu(C) videobuf2_dma_contig v4l2_mem2mem
[  114.375749] Process ffmpeg (pid: 1871, stack limit = 0x0000000059d846e4)
[  114.382450] CPU: 1 PID: 1871 Comm: ffmpeg Tainted: G         C        5.1.16-20190703-1 #2
[  114.390710] Hardware name: NXP i.MX8MQ EVK (DT)
[  114.395240] pstate: 40000005 (nZcv daif -PAN -UAO)
[  114.400042] pc : hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.406139] lr : hantro_g1_vp8_dec_run+0x1160/0x18a0 [hantro_vpu]
[  114.412229] sp : ffff000011ae3c10
[  114.415541] x29: ffff000011ae3c10 x28: ffff000008a154c8
[  114.420853] x27: 000000007033b039 x26: ffff000008a130f0
[  114.426164] x25: 000000000000000c x24: ffff000008a153f0
[  114.431474] x23: ffff800099a0d880 x22: ffff000008a13150
[  114.436785] x21: 000000000c5b88d0 x20: ffff80009b7d65a0
[  114.442096] x19: ffff800099bd3800 x18: 0000000000000010
[  114.447407] x17: 0000000000000001 x16: 0000000000000007
[  114.452717] x15: ffffffffffffffff x14: ffff000010e8c5c8
[  114.458028] x13: ffff000091ae3987 x12: ffff0000112b0002
[  114.463339] x11: ffff000010ea4000 x10: ffff000011ae3910
[  114.468649] x9 : 00000000ffffffd0 x8 : 00000000edcb88d0
[  114.473960] x7 : 0000000000000125 x6 : ffff000010e8cd60
[  114.479270] x5 : ffff000010e8c000 x4 : 0000000000000000
[  114.484580] x3 : 0000000000000002 x2 : 8127d140a3196d00
[  114.489891] x1 : 0000000000000000 x0 : 00000000e1700000
[  114.495201] Call trace:
[  114.497652]  hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.503401]  device_run+0xac/0xc0 [hantro_vpu]
[  114.507849]  v4l2_m2m_try_run+0x9c/0x110 [v4l2_mem2mem]
[  114.513077]  v4l2_m2m_request_queue+0xd4/0x130 [v4l2_mem2mem]
[  114.518826]  media_request_ioctl+0x1e8/0x2d0
[  114.523097]  do_vfs_ioctl+0xc4/0x870
[  114.526671]  ksys_ioctl+0x84/0xc0
[  114.529985]  __arm64_sys_ioctl+0x28/0x40
[  114.533908]  el0_svc_common.constprop.0+0x98/0x170
[  114.538698]  el0_svc_handler+0x2c/0x40
[  114.542447]  el0_svc+0x8/0xc
[  114.545328] Code: 0b150008 b94002c3 121d7108 8b23418c (b940018c)
[  114.551421] ---[ end trace b9ad6b0f72902ba5 ]---

Quoted 17 lines

---
Changes from v1:
* Place operators at the end of each line.
* Update to uAPI changes.

---
 drivers/staging/media/hantro/Makefile         |   4 +-
 drivers/staging/media/hantro/hantro.h         |   5 +
 drivers/staging/media/hantro/hantro_drv.c     |   6 +
 .../staging/media/hantro/hantro_g1_vp8_dec.c  | 552 ++++++++++++++++++
 drivers/staging/media/hantro/hantro_hw.h      |  17 +
 drivers/staging/media/hantro/hantro_v4l2.c    |   1 +
 drivers/staging/media/hantro/hantro_vp8.c     | 188 ++++++
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |  22 +-
 8 files changed, 793 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_vp8.c

[...]

Quoted 6 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +0 -0

new file mode 100644
index 000000000000..31d31faae4aa
@@ -0,0 +1,552 @@  

[...]

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition base address regs */
+static const struct vp8_dec_reg vp8_dec_dct_base[8] = {  

[...]

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 81

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition start bits regs */
+static const struct vp8_dec_reg vp8_dec_dct_start_bits[8] = {  

So these arrays can be directly indexed with values smaller than 8 ...

[...]

Quoted 3 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +3 -0

+static void cfg_parts(struct hantro_ctx *ctx,
+		      const struct v4l2_ctrl_vp8_frame_header *hdr)
+{  

[...]

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 358

Quoted 10 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +10 -0

+	/* dct partitions base address */
+	for (i = 0; i < hdr->num_dct_parts; i++) {
+		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
+		u32 base_addr = byte_offset + src_dma;
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_base[i],
+				  base_addr & (~DEC_8190_ALIGN_MASK));
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_start_bits[i],
+				  (byte_offset & DEC_8190_ALIGN_MASK) * 8);  

... and here they are indexed with i, which is only guaranteed to be smaller than hdr->num_dct_parts. num_dct_parts is passed from userspace via v4l2-ctrl, it can be as large as 255.

+Kees for the safe-array-iteratio question.

Quoted 103 lines · On Wed, 03 Jul 2019 16:26:46 +0200 Philipp Zabel <[email protected]> wrote:

Hi Ezequiel

Quoted 8 lines · Ezequiel Garcia

From: ZhiChao Yu <[email protected]>

Introduce VP8 decoding support in RK3288.

Signed-off-by ZhiChao Yu <[email protected]>

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

I have just tried this (with broken userspace) and got a crash in cfg_parts, see below for details:

[  114.308757] Unable to handle kernel paging request at virtual address ffff0000112b0002
[  114.316691] Mem abort info:
[  114.319503]   ESR = 0x96000021
[  114.322576]   Exception class = DABT (current EL), IL = 32 bits
[  114.328513]   SET = 0, FnV = 0
[  114.331586]   EA = 0, S1PTW = 0
[  114.334744] Data abort info:
[  114.337626]   ISV = 0, ISS = 0x00000021
[  114.341479]   CM = 0, WnR = 0
[  114.344466] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000040d61000
[  114.351185] [ffff0000112b0002] pgd=00000000dffff003, pud=00000000dfffe003, pmd=00000000dbf36003, pte=00e8000038300707
[  114.361822] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[  114.367394] Modules linked in: crct10dif_ce hantro_vpu(C) videobuf2_dma_contig v4l2_mem2mem
[  114.375749] Process ffmpeg (pid: 1871, stack limit = 0x0000000059d846e4)
[  114.382450] CPU: 1 PID: 1871 Comm: ffmpeg Tainted: G         C        5.1.16-20190703-1 #2
[  114.390710] Hardware name: NXP i.MX8MQ EVK (DT)
[  114.395240] pstate: 40000005 (nZcv daif -PAN -UAO)
[  114.400042] pc : hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.406139] lr : hantro_g1_vp8_dec_run+0x1160/0x18a0 [hantro_vpu]
[  114.412229] sp : ffff000011ae3c10
[  114.415541] x29: ffff000011ae3c10 x28: ffff000008a154c8 
[  114.420853] x27: 000000007033b039 x26: ffff000008a130f0 
[  114.426164] x25: 000000000000000c x24: ffff000008a153f0 
[  114.431474] x23: ffff800099a0d880 x22: ffff000008a13150 
[  114.436785] x21: 000000000c5b88d0 x20: ffff80009b7d65a0 
[  114.442096] x19: ffff800099bd3800 x18: 0000000000000010 
[  114.447407] x17: 0000000000000001 x16: 0000000000000007 
[  114.452717] x15: ffffffffffffffff x14: ffff000010e8c5c8 
[  114.458028] x13: ffff000091ae3987 x12: ffff0000112b0002 
[  114.463339] x11: ffff000010ea4000 x10: ffff000011ae3910 
[  114.468649] x9 : 00000000ffffffd0 x8 : 00000000edcb88d0 
[  114.473960] x7 : 0000000000000125 x6 : ffff000010e8cd60 
[  114.479270] x5 : ffff000010e8c000 x4 : 0000000000000000 
[  114.484580] x3 : 0000000000000002 x2 : 8127d140a3196d00 
[  114.489891] x1 : 0000000000000000 x0 : 00000000e1700000 
[  114.495201] Call trace:
[  114.497652]  hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.503401]  device_run+0xac/0xc0 [hantro_vpu]
[  114.507849]  v4l2_m2m_try_run+0x9c/0x110 [v4l2_mem2mem]
[  114.513077]  v4l2_m2m_request_queue+0xd4/0x130 [v4l2_mem2mem]
[  114.518826]  media_request_ioctl+0x1e8/0x2d0
[  114.523097]  do_vfs_ioctl+0xc4/0x870
[  114.526671]  ksys_ioctl+0x84/0xc0
[  114.529985]  __arm64_sys_ioctl+0x28/0x40
[  114.533908]  el0_svc_common.constprop.0+0x98/0x170
[  114.538698]  el0_svc_handler+0x2c/0x40
[  114.542447]  el0_svc+0x8/0xc
[  114.545328] Code: 0b150008 b94002c3 121d7108 8b23418c (b940018c) 
[  114.551421] ---[ end trace b9ad6b0f72902ba5 ]---

Quoted 17 lines

---
Changes from v1:
* Place operators at the end of each line.
* Update to uAPI changes.

---
 drivers/staging/media/hantro/Makefile         |   4 +-
 drivers/staging/media/hantro/hantro.h         |   5 +
 drivers/staging/media/hantro/hantro_drv.c     |   6 +
 .../staging/media/hantro/hantro_g1_vp8_dec.c  | 552 ++++++++++++++++++
 drivers/staging/media/hantro/hantro_hw.h      |  17 +
 drivers/staging/media/hantro/hantro_v4l2.c    |   1 +
 drivers/staging/media/hantro/hantro_vp8.c     | 188 ++++++
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |  22 +-
 8 files changed, 793 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_vp8.c

[...]

Quoted 6 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +0 -0

new file mode 100644
index 000000000000..31d31faae4aa
@@ -0,0 +1,552 @@  

[...]

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition base address regs */
+static const struct vp8_dec_reg vp8_dec_dct_base[8] = {  

[...]

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 81

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition start bits regs */
+static const struct vp8_dec_reg vp8_dec_dct_start_bits[8] = {  

So these arrays can be directly indexed with values smaller than 8 ...

[...]

Quoted 3 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +3 -0

+static void cfg_parts(struct hantro_ctx *ctx,
+		      const struct v4l2_ctrl_vp8_frame_header *hdr)
+{  

[...]

Quoted 14 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 358

Quoted 10 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +10 -0

+	/* dct partitions base address */
+	for (i = 0; i < hdr->num_dct_parts; i++) {
+		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
+		u32 base_addr = byte_offset + src_dma;
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_base[i],
+				  base_addr & (~DEC_8190_ALIGN_MASK));
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_start_bits[i],
+				  (byte_offset & DEC_8190_ALIGN_MASK) * 8);  

... and here they are indexed with i, which is only guaranteed to be smaller than hdr->num_dct_parts. num_dct_parts is passed from userspace via v4l2-ctrl, it can be as large as 255.

Hm, I fear we have the same problem in other places (including the patch series adding support for H264). Kees, I wonder if there's some kind of safe array iterator macro, something like

#define for_each_static_array_entry_safe(_array, _iter, _max_user) 		\
	_max_user = min_t(typeof(_max_user), _max_user,	ARRAY_SIZE(_array));	\
	for (_iter = 0; _iter < _max_user; _iter++)

The problem with this approach is that it's papering over the real issue, which is that hdr->num_dct_parts should be checked and the driver/core should return an error when it's > 7 instead of silently iterating over the 8 entries of the dct[] arrays. Static code analysis tools can probably detect such issues too.

Quoted 131 lines · Boris Brezillon

+Kees for the safe-array-iteratio question.

Quoted 103 lines · On Wed, 03 Jul 2019 16:26:46 +0200 Philipp Zabel <[email protected]> wrote:

Hi Ezequiel

Quoted 8 lines · Ezequiel Garcia

From: ZhiChao Yu <[email protected]>

Introduce VP8 decoding support in RK3288.

Signed-off-by ZhiChao Yu <[email protected]>

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

I have just tried this (with broken userspace) and got a crash in cfg_parts, see below for details:

[  114.308757] Unable to handle kernel paging request at virtual address ffff0000112b0002
[  114.316691] Mem abort info:
[  114.319503]   ESR = 0x96000021
[  114.322576]   Exception class = DABT (current EL), IL = 32 bits
[  114.328513]   SET = 0, FnV = 0
[  114.331586]   EA = 0, S1PTW = 0
[  114.334744] Data abort info:
[  114.337626]   ISV = 0, ISS = 0x00000021
[  114.341479]   CM = 0, WnR = 0
[  114.344466] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000040d61000
[  114.351185] [ffff0000112b0002] pgd=00000000dffff003, pud=00000000dfffe003, pmd=00000000dbf36003, pte=00e8000038300707
[  114.361822] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[  114.367394] Modules linked in: crct10dif_ce hantro_vpu(C) videobuf2_dma_contig v4l2_mem2mem
[  114.375749] Process ffmpeg (pid: 1871, stack limit = 0x0000000059d846e4)
[  114.382450] CPU: 1 PID: 1871 Comm: ffmpeg Tainted: G         C        5.1.16-20190703-1 #2
[  114.390710] Hardware name: NXP i.MX8MQ EVK (DT)
[  114.395240] pstate: 40000005 (nZcv daif -PAN -UAO)
[  114.400042] pc : hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.406139] lr : hantro_g1_vp8_dec_run+0x1160/0x18a0 [hantro_vpu]
[  114.412229] sp : ffff000011ae3c10
[  114.415541] x29: ffff000011ae3c10 x28: ffff000008a154c8 
[  114.420853] x27: 000000007033b039 x26: ffff000008a130f0 
[  114.426164] x25: 000000000000000c x24: ffff000008a153f0 
[  114.431474] x23: ffff800099a0d880 x22: ffff000008a13150 
[  114.436785] x21: 000000000c5b88d0 x20: ffff80009b7d65a0 
[  114.442096] x19: ffff800099bd3800 x18: 0000000000000010 
[  114.447407] x17: 0000000000000001 x16: 0000000000000007 
[  114.452717] x15: ffffffffffffffff x14: ffff000010e8c5c8 
[  114.458028] x13: ffff000091ae3987 x12: ffff0000112b0002 
[  114.463339] x11: ffff000010ea4000 x10: ffff000011ae3910 
[  114.468649] x9 : 00000000ffffffd0 x8 : 00000000edcb88d0 
[  114.473960] x7 : 0000000000000125 x6 : ffff000010e8cd60 
[  114.479270] x5 : ffff000010e8c000 x4 : 0000000000000000 
[  114.484580] x3 : 0000000000000002 x2 : 8127d140a3196d00 
[  114.489891] x1 : 0000000000000000 x0 : 00000000e1700000 
[  114.495201] Call trace:
[  114.497652]  hantro_g1_vp8_dec_run+0x1178/0x18a0 [hantro_vpu]
[  114.503401]  device_run+0xac/0xc0 [hantro_vpu]
[  114.507849]  v4l2_m2m_try_run+0x9c/0x110 [v4l2_mem2mem]
[  114.513077]  v4l2_m2m_request_queue+0xd4/0x130 [v4l2_mem2mem]
[  114.518826]  media_request_ioctl+0x1e8/0x2d0
[  114.523097]  do_vfs_ioctl+0xc4/0x870
[  114.526671]  ksys_ioctl+0x84/0xc0
[  114.529985]  __arm64_sys_ioctl+0x28/0x40
[  114.533908]  el0_svc_common.constprop.0+0x98/0x170
[  114.538698]  el0_svc_handler+0x2c/0x40
[  114.542447]  el0_svc+0x8/0xc
[  114.545328] Code: 0b150008 b94002c3 121d7108 8b23418c (b940018c) 
[  114.551421] ---[ end trace b9ad6b0f72902ba5 ]---

Quoted 17 lines

---
Changes from v1:
* Place operators at the end of each line.
* Update to uAPI changes.

---
 drivers/staging/media/hantro/Makefile         |   4 +-
 drivers/staging/media/hantro/hantro.h         |   5 +
 drivers/staging/media/hantro/hantro_drv.c     |   6 +
 .../staging/media/hantro/hantro_g1_vp8_dec.c  | 552 ++++++++++++++++++
 drivers/staging/media/hantro/hantro_hw.h      |  17 +
 drivers/staging/media/hantro/hantro_v4l2.c    |   1 +
 drivers/staging/media/hantro/hantro_vp8.c     | 188 ++++++
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |  22 +-
 8 files changed, 793 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
 create mode 100644 drivers/staging/media/hantro/hantro_vp8.c

[...]

Quoted 6 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +0 -0

new file mode 100644
index 000000000000..31d31faae4aa
@@ -0,0 +1,552 @@    

[...]

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition base address regs */
+static const struct vp8_dec_reg vp8_dec_dct_base[8] = {    

[...]

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 81

Quoted 2 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +2 -0

+/* dct partition start bits regs */
+static const struct vp8_dec_reg vp8_dec_dct_start_bits[8] = {    

So these arrays can be directly indexed with values smaller than 8 ...

[...]

Quoted 3 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +3 -0

+static void cfg_parts(struct hantro_ctx *ctx,
+		      const struct v4l2_ctrl_vp8_frame_header *hdr)
+{    

[...]

Quoted 14 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c line 358

Quoted 10 lines

drivers/staging/media/hantro/hantro_g1_vp8_dec.c new file +10 -0

+	/* dct partitions base address */
+	for (i = 0; i < hdr->num_dct_parts; i++) {
+		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
+		u32 base_addr = byte_offset + src_dma;
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_base[i],
+				  base_addr & (~DEC_8190_ALIGN_MASK));
+
+		vp8_dec_reg_write(vpu, &vp8_dec_dct_start_bits[i],
+				  (byte_offset & DEC_8190_ALIGN_MASK) * 8);    

... and here they are indexed with i, which is only guaranteed to be smaller than hdr->num_dct_parts. num_dct_parts is passed from userspace via v4l2-ctrl, it can be as large as 255.

Hm, I fear we have the same problem in other places (including the patch series adding support for H264). Kees, I wonder if there's some kind of safe array iterator macro, something like

#define for_each_static_array_entry_safe(_array, _iter, _max_user) 		\
	_max_user = min_t(typeof(_max_user), _max_user,	ARRAY_SIZE(_array));	\
	for (_iter = 0; _iter < _max_user; _iter++)

Quoted 6 lines

The problem with this approach is that it's papering over the real issue, which is that hdr->num_dct_parts should be checked and the driver/core should return an error when it's > 7 instead of silently iterating over the 8 entries of the dct[] arrays. Static code analysis tools can probably detect such issues too.

It seems to me that drivers/media/v4l2-core/v4l2-ctrls.c:std_validate is the right place for these sanity checks.

And so drivers can then assume safe values on the controls.

Hm, I fear we have the same problem in other places (including the patch series adding support for H264). Kees, I wonder if there's some kind of safe array iterator macro, something like

#define for_each_static_array_entry_safe(_array, _iter, _max_user) 		\
	_max_user = min_t(typeof(_max_user), _max_user,	ARRAY_SIZE(_array));	\
	for (_iter = 0; _iter < _max_user; _iter++)

The problem with this approach is that it's papering over the real issue, which is that hdr->num_dct_parts should be checked and the driver/core should return an error when it's > 7 instead of silently iterating over the 8 entries of the dct[] arrays. Static code analysis tools can probably detect such issues too.

Any advice on how to detect such problems early on?