Skip to contentSkip to conversation

media: uapi: Add VP8 low-level decoder API compound controls.

RFC In review Ezequiel Garcia 4 participants 9 messages Feb 13, 2019 – Jun 27, 2019 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

RFC 1/1 media: uapi: Add VP8 low-level decoder API compound controls.

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +7 -0
index 366200d31bc0..c77a56c3e2aa 100644
@@ -869,6 +869,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_HDR:			return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1694,6 +1698,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_HDR:
 		return 0;
 
 	default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 		break;
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+	case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
index c765c7c7c562..ea295aa9d0b6 100644
@@ -1324,6 +1324,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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:		Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:	Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
 	struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 
include/media/vp8-ctrls.h new file +104 -0

104 lines not on this page · raw message

include/uapi/linux/videodev2.h +1 -0
index f6a484017208..a906bfc0c8f0 100644
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
Signature · 1 line
2.20.1
#1 lore raw

Hi,

Quoted 106 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +7 -0
index 366200d31bc0..c77a56c3e2aa 100644
@@ -869,6 +869,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_HDR:			return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1694,6 +1698,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_HDR:
 		return 0;
 
 	default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 		break;
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+	case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
index c765c7c7c562..ea295aa9d0b6 100644
@@ -1324,6 +1324,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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:		Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:	Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
 	struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 
include/media/vp8-ctrls.h new file +0 -0
new file mode 100644
index 000000000000..95b63a0cb239

include/media/vp8-ctrls.h line 3

Quoted 3 lines
include/media/vp8-ctrls.h new file +3 -0
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.

This is still pending.

Quoted 47 lines
include/media/vp8-ctrls.h new file +47 -0
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR		0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+	__u8 segment_feature_mode;
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE	0x02
+struct v4l2_vp8_loopfilter_header {
+	__u16 type;
+	__u8 level;
+	__u8 sharpness_level;
+	__s8 ref_frm_delta_magnitude[4];
+	__s8 mb_mode_delta_magnitude[4];
+	__u16 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 dequant_factors[4][3][2];
+};
+
+struct v4l2_vp8_entropy_header {
+	__u8 coeff_probs[4][8][3][11];
+	__u8 y_mode_probs[4];

include/media/vp8-ctrls.h line 60

Quoted 10 lines
include/media/vp8-ctrls.h new file +10 -0
+	__u8 uv_mode_probs[3];
+	__u8 mv_probs[2][19];
+};
+
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL		0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME		0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF	0x04
+struct v4l2_ctrl_vp8_frame_header {
+	/* 0: keyframe, 1: not a keyframe */
+	__u8 key_frame; // could be a flag?

Is there any reason why there is a separate field for key_frame?

Quoted 13 lines
include/media/vp8-ctrls.h new file +13 -0
+	__u8 version;
+
+	/* Populated also if not a key frame */
+	__u16 width;
+	__u16 height;
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	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;
+

include/media/vp8-ctrls.h line 83

Quoted 10 lines
include/media/vp8-ctrls.h new file +10 -0
+	__u8 sign_bias_golden;
+	__u8 sign_bias_alternate;
+
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+
+	__u32 first_part_size;
+	__u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;

As the comment says, it seems the first partition offset is always 3 + 7 * s->keyframe. Or am I wrong?

Quoted 33 lines
include/media/vp8-ctrls.h new file +21 -0
+	/*
+	 * Offset in bits of MB data in first partition,
+	 * i.e. bit offset starting from first_part_offset.
+	 */
+	__u32 macroblock_bit_offset;
+
+	__u8 num_dct_parts;
+	__u32 dct_part_sizes[8];
+
+	__u8 bool_dec_range;
+	__u8 bool_dec_value;
+	__u8 bool_dec_count;
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+
+	__u8 flags;
+};
+
+#endif
include/uapi/linux/videodev2.h +1 -0
index f6a484017208..a906bfc0c8f0 100644
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
#2 lore raw
Quoted 167 lines · Ezequiel Garcia

Hi,

Quoted 106 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +7 -0
index 366200d31bc0..c77a56c3e2aa 100644
@@ -869,6 +869,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_HDR:                 return "VP8 Frame Header";
 
      /* HEVC controls */
      case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:               return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+             *type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+             break;
      default:
              *type = V4L2_CTRL_TYPE_INTEGER;
              break;
@@ -1694,6 +1698,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_HDR:
              return 0;
 
      default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
              break;
      case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
              elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+     case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+             elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
              break;
      default:
              if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
index c765c7c7c562..ea295aa9d0b6 100644
@@ -1324,6 +1324,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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:   Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:              Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:     Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:              Pointer to a VP8 frame header structure.
  * @p:                               Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
      struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
      struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
      struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+     struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
      void *p;
 };
 
include/media/vp8-ctrls.h new file +0 -0
new file mode 100644
index 000000000000..95b63a0cb239

include/media/vp8-ctrls.h line 3

Quoted 3 lines
include/media/vp8-ctrls.h new file +3 -0
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.

This is still pending.

Quoted 51 lines
include/media/vp8-ctrls.h new file +51 -0
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR         0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+     __u8 segment_feature_mode;
+     __s8 quant_update[4];
+     __s8 lf_update[4];
+     __u8 segment_probs[3];
+     __u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE   0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE 0x02
+struct v4l2_vp8_loopfilter_header {
+     __u16 type;
+     __u8 level;
+     __u8 sharpness_level;
+     __s8 ref_frm_delta_magnitude[4];
+     __s8 mb_mode_delta_magnitude[4];
+     __u16 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 dequant_factors[4][3][2];
+};
+
+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];
+};
+
Quoted 8 lines

include/media/vp8-ctrls.h line 60

Quoted 6 lines
include/media/vp8-ctrls.h new file +6 -0
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL         0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME           0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF     0x04
+struct v4l2_ctrl_vp8_frame_header {
+     /* 0: keyframe, 1: not a keyframe */
+     __u8 key_frame; // could be a flag?

Is there any reason why there is a separate field for key_frame?

This is exposed differently in VA VAPI, not flag because it's harder to use.
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L91

Quoted 19 lines
Quoted 19 lines
include/media/vp8-ctrls.h new file +18 -0
+     __u8 version;
+
+     /* Populated also if not a key frame */
+     __u16 width;
+     __u16 height;
+     __u8 horizontal_scale;
+     __u8 vertical_scale;
+
+     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;
+
+     __u8 sign_bias_golden;
+     __u8 sign_bias_alternate;
+
+     __u8 prob_skip_false;
+     __u8 prob_intra;
Quoted 8 lines

include/media/vp8-ctrls.h line 83

Quoted 5 lines
include/media/vp8-ctrls.h new file +5 -0
+     __u8 prob_last;
+     __u8 prob_gf;
+
+     __u32 first_part_size;
+     __u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;

As the comment says, it seems the first partition offset is always 3 + 7 * s->keyframe. Or am I wrong?

I can't find it in VA API or GStreamer parsers. Ideally we need to look in the spec, if it's calculated it does not belong here.

https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/gst-libs/gst/codecparsers/gstvp8parser.h#L255
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L72

Notice that VA splits this in two, the some part in the picture parameter, and some parts as SliceParameters. I believe it's to avoid having conditional field base on if key_frame == 0.

Quoted 36 lines
Quoted 34 lines
include/media/vp8-ctrls.h new file +21 -0
+     /*
+      * Offset in bits of MB data in first partition,
+      * i.e. bit offset starting from first_part_offset.
+      */
+     __u32 macroblock_bit_offset;
+
+     __u8 num_dct_parts;
+     __u32 dct_part_sizes[8];
+
+     __u8 bool_dec_range;
+     __u8 bool_dec_value;
+     __u8 bool_dec_count;
+
+     __u64 last_frame_ts;
+     __u64 golden_frame_ts;
+     __u64 alt_frame_ts;
+
+     __u8 flags;
+};
+
+#endif
include/uapi/linux/videodev2.h +1 -0
index f6a484017208..a906bfc0c8f0 100644
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
#3 lore raw
Quoted 177 lines · Nicolas Dufresne
Quoted 166 lines · Ezequiel Garcia

Hi,

Quoted 106 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +7 -0
index 366200d31bc0..c77a56c3e2aa 100644
@@ -869,6 +869,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_HDR:                 return "VP8 Frame Header";
 
      /* HEVC controls */
      case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:               return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+             *type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+             break;
      default:
              *type = V4L2_CTRL_TYPE_INTEGER;
              break;
@@ -1694,6 +1698,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_HDR:
              return 0;
 
      default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
              break;
      case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
              elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+     case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+             elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
              break;
      default:
              if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
index c765c7c7c562..ea295aa9d0b6 100644
@@ -1324,6 +1324,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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:   Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:              Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:     Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:              Pointer to a VP8 frame header structure.
  * @p:                               Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
      struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
      struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
      struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+     struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
      void *p;
 };
 
include/media/vp8-ctrls.h new file +0 -0
new file mode 100644
index 000000000000..95b63a0cb239

include/media/vp8-ctrls.h line 3

Quoted 3 lines
include/media/vp8-ctrls.h new file +3 -0
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.

This is still pending.

Quoted 51 lines
include/media/vp8-ctrls.h new file +51 -0
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR         0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+     __u8 segment_feature_mode;
+     __s8 quant_update[4];
+     __s8 lf_update[4];
+     __u8 segment_probs[3];
+     __u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE   0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE 0x02
+struct v4l2_vp8_loopfilter_header {
+     __u16 type;
+     __u8 level;
+     __u8 sharpness_level;
+     __s8 ref_frm_delta_magnitude[4];
+     __s8 mb_mode_delta_magnitude[4];
+     __u16 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 dequant_factors[4][3][2];
+};
+
+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];
+};
+
Quoted 8 lines

include/media/vp8-ctrls.h line 60

Quoted 6 lines
include/media/vp8-ctrls.h new file +6 -0
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL         0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME           0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF     0x04
+struct v4l2_ctrl_vp8_frame_header {
+     /* 0: keyframe, 1: not a keyframe */
+     __u8 key_frame; // could be a flag?

Is there any reason why there is a separate field for key_frame?

Quoted 3 lines

This is exposed differently in VA VAPI, not flag because it's harder to use.
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L91

Well, it is exposed with a bit field. However, bit fields are rarely used (if at all) in the uAPI.

Why do you say it's harder to use?

if (a.flags & KEYFRAME)

vs.

if (a.keyframe)

The way I see it, it's either all flags or all fields. Although I might be missing something.

Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
include/media/vp8-ctrls.h new file +18 -0
+     __u8 version;
+
+     /* Populated also if not a key frame */
+     __u16 width;
+     __u16 height;
+     __u8 horizontal_scale;
+     __u8 vertical_scale;
+
+     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;
+
+     __u8 sign_bias_golden;
+     __u8 sign_bias_alternate;
+
+     __u8 prob_skip_false;
+     __u8 prob_intra;
Quoted 8 lines

include/media/vp8-ctrls.h line 83

Quoted 5 lines
include/media/vp8-ctrls.h new file +5 -0
+     __u8 prob_last;
+     __u8 prob_gf;
+
+     __u32 first_part_size;
+     __u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;

As the comment says, it seems the first partition offset is always 3 + 7 * s->keyframe. Or am I wrong?

Quoted 3 lines

I can't find it in VA API or GStreamer parsers. Ideally we need to look in the spec, if it's calculated it does not belong here.

Looking into the spec, I don't think it's part of it.

Quoted 7 lines

https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/gst-libs/gst/codecparsers/gstvp8parser.h#L255
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L72

Notice that VA splits this in two, the some part in the picture parameter, and some parts as SliceParameters. I believe it's to avoid having conditional field base on if key_frame == 0.

That might make sense. Something to look into.

Thanks for the review!
Ezequiel

#4 lore raw
Quoted 7 lines · Ezequiel Garcia
Quoted 5 lines · Nicolas Dufresne
Quoted 3 lines · Ezequiel Garcia

Hi,

On Wed, 2019-02-13 at 18:15 -0300, Ezequiel Garcia wrote:

[snip]

Quoted 36 lines
Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
include/media/vp8-ctrls.h new file +18 -0
+     __u8 version;
+
+     /* Populated also if not a key frame */
+     __u16 width;
+     __u16 height;
+     __u8 horizontal_scale;
+     __u8 vertical_scale;
+
+     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;
+
+     __u8 sign_bias_golden;
+     __u8 sign_bias_alternate;
+
+     __u8 prob_skip_false;
+     __u8 prob_intra;
Quoted 8 lines

include/media/vp8-ctrls.h line 83

Quoted 5 lines
include/media/vp8-ctrls.h new file +5 -0
+     __u8 prob_last;
+     __u8 prob_gf;
+
+     __u32 first_part_size;
+     __u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;

As the comment says, it seems the first partition offset is always 3 + 7 * s->keyframe. Or am I wrong?

Quoted 3 lines

I can't find it in VA API or GStreamer parsers. Ideally we need to look in the spec, if it's calculated it does not belong here.

Looking into the spec, I don't think it's part of it.

Quoted 3 lines

https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/gst-libs/gst/codecparsers/gstvp8parser.h#L255
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L72

Quoted 6 lines
Quoted 4 lines

Notice that VA splits this in two, the some part in the picture parameter, and some parts as SliceParameters. I believe it's to avoid having conditional field base on if key_frame == 0.

That might make sense. Something to look into.

Yeah, sounds reasonable, although VAAPI naming is a bit off, since VP8 doesn't have a notion of slices...

Best regards,
Tomasz

#5 lore raw
Quoted 49 lines · Tomasz Figa
Quoted 6 lines · Ezequiel Garcia
Quoted 5 lines · Nicolas Dufresne
Quoted 3 lines · Ezequiel Garcia

Hi,

On Wed, 2019-02-13 at 18:15 -0300, Ezequiel Garcia wrote:

[snip]

Quoted 41 lines
Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
include/media/vp8-ctrls.h new file +18 -0
+     __u8 version;
+
+     /* Populated also if not a key frame */
+     __u16 width;
+     __u16 height;
+     __u8 horizontal_scale;
+     __u8 vertical_scale;
+
+     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;
+
+     __u8 sign_bias_golden;
+     __u8 sign_bias_alternate;
+
+     __u8 prob_skip_false;
+     __u8 prob_intra;
Quoted 8 lines

include/media/vp8-ctrls.h line 83

Quoted 5 lines
include/media/vp8-ctrls.h new file +5 -0
+     __u8 prob_last;
+     __u8 prob_gf;
+
+     __u32 first_part_size;
+     __u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;

As the comment says, it seems the first partition offset is always 3 + 7 * s->keyframe. Or am I wrong?

Quoted 3 lines

I can't find it in VA API or GStreamer parsers. Ideally we need to look in the spec, if it's calculated it does not belong here.

Looking into the spec, I don't think it's part of it.

Quoted 7 lines

https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/gst-libs/gst/codecparsers/gstvp8parser.h#L255
https://github.com/intel/libva/blob/master/va/va_dec_vp8.h#L72

Notice that VA splits this in two, the some part in the picture parameter, and some parts as SliceParameters. I believe it's to avoid having conditional field base on if key_frame == 0.

Quoted 4 lines
Quoted 1 line

That might make sense. Something to look into.

Yeah, sounds reasonable, although VAAPI naming is a bit off, since VP8 doesn't have a notion of slices...

Agreed, VAAPI uses slices to fit their abstraction. VP8 is just a special case where you only have one slice per frame. It's not needed in this uAPI for the kernel.

Quoted 3 lines

Best regards,
Tomasz

#6 lore raw
In reply to Ezequiel Garcia
Quoted 156 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +7 -0
index 366200d31bc0..c77a56c3e2aa 100644
@@ -869,6 +869,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_HDR:			return "VP8 Frame Header";
 
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1694,6 +1698,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_HDR:
 		return 0;
 
 	default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 		break;
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+	case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
index c765c7c7c562..ea295aa9d0b6 100644
@@ -1324,6 +1324,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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:		Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:	Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
 	struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 
include/media/vp8-ctrls.h new file +50 -0
new file mode 100644
index 000000000000..95b63a0cb239
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR		0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+	__u8 segment_feature_mode;
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE	0x02
+struct v4l2_vp8_loopfilter_header {
+	__u16 type;
+	__u8 level;
+	__u8 sharpness_level;
+	__s8 ref_frm_delta_magnitude[4];
+	__s8 mb_mode_delta_magnitude[4];
+	__u16 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 dequant_factors[4][3][2];
+};
+
+struct v4l2_vp8_entropy_header {
+	__u8 coeff_probs[4][8][3][11];
+	__u8 y_mode_probs[4];

include/media/vp8-ctrls.h line 60

Quoted 10 lines
include/media/vp8-ctrls.h new file +10 -0
+	__u8 uv_mode_probs[3];
+	__u8 mv_probs[2][19];
+};
+
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL		0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME		0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF	0x04
+struct v4l2_ctrl_vp8_frame_header {
+	/* 0: keyframe, 1: not a keyframe */
+	__u8 key_frame; // could be a flag?

Yes, I think it would be more consistent to make this a flag.

Quoted 25 lines
include/media/vp8-ctrls.h new file +25 -0
+	__u8 version;
+
+	/* Populated also if not a key frame */
+	__u16 width;
+	__u16 height;
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	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;
+
+	__u8 sign_bias_golden;
+	__u8 sign_bias_alternate;
+
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+
+	__u32 first_part_size;
+	__u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;
+	/*
+	 * Offset in bits of MB data in first partition,

include/media/vp8-ctrls.h line 95

Quoted 10 lines
include/media/vp8-ctrls.h new file +10 -0
+	 * i.e. bit offset starting from first_part_offset.
+	 */
+	__u32 macroblock_bit_offset;
+
+	__u8 num_dct_parts;
+	__u32 dct_part_sizes[8];
+
+	__u8 bool_dec_range;
+	__u8 bool_dec_value;
+	__u8 bool_dec_count;

I don't know if we care about packing in general, but you have 3 bytes in between a __u32 and a __u64.

Quoted 21 lines
include/media/vp8-ctrls.h new file +9 -0
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+
+	__u8 flags;
+};
+
+#endif
include/uapi/linux/videodev2.h +1 -0
index f6a484017208..a906bfc0c8f0 100644
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
#7 lore raw
In reply to Ezequiel Garcia

Hi,

Quoted 26 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +0 -0
index 366200d31bc0..c77a56c3e2aa 100644

drivers/media/v4l2-core/v4l2-ctrls.c const char *v4l2_ctrl_get_name(u32 id) · line 872

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ctrls.c +1 -0
@@ -869,6 +869,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_HDR:			return "VP8 Frame Header";

Could we call this _FRAME_HEADER to match the control name?

I find it quite handy to have the ctrl definitions and associated structures named in sync.

Quoted 35 lines
drivers/media/v4l2-core/v4l2-ctrls.c +6 -0
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1694,6 +1698,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_HDR:
 		return 0;
 
 	default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 		break;
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+	case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +0 -0
index c765c7c7c562..ea295aa9d0b6 100644

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1327

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
@@ -1324,6 +1324,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;

I'm not familiar with VP8 at all, but are we guaranteed that only per-frame decoding mode is possible?

Quoted 146 lines
drivers/media/v4l2-core/v4l2-ioctl.c +0 -0
 		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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:		Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:	Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
 	struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 
include/media/vp8-ctrls.h new file +104 -0
new file mode 100644
index 000000000000..95b63a0cb239
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR		0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+	__u8 segment_feature_mode;
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE	0x02
+struct v4l2_vp8_loopfilter_header {
+	__u16 type;
+	__u8 level;
+	__u8 sharpness_level;
+	__s8 ref_frm_delta_magnitude[4];
+	__s8 mb_mode_delta_magnitude[4];
+	__u16 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 dequant_factors[4][3][2];
+};
+
+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];
+};
+
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL		0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME		0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF	0x04
+struct v4l2_ctrl_vp8_frame_header {
+	/* 0: keyframe, 1: not a keyframe */
+	__u8 key_frame; // could be a flag?
+	__u8 version;
+
+	/* Populated also if not a key frame */
+	__u16 width;
+	__u16 height;
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	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;
+
+	__u8 sign_bias_golden;
+	__u8 sign_bias_alternate;
+
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+
+	__u32 first_part_size;
+	__u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;
+	/*
+	 * Offset in bits of MB data in first partition,
+	 * i.e. bit offset starting from first_part_offset.
+	 */
+	__u32 macroblock_bit_offset;
+
+	__u8 num_dct_parts;
+	__u32 dct_part_sizes[8];
+
+	__u8 bool_dec_range;
+	__u8 bool_dec_value;
+	__u8 bool_dec_count;
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+
+	__u8 flags;
+};
+
+#endif
include/uapi/linux/videodev2.h +0 -0
index f6a484017208..a906bfc0c8f0 100644

include/uapi/linux/videodev2.h struct v4l2_pix_format { · line 667

Quoted 4 lines
include/uapi/linux/videodev2.h +1 -0
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */

Could we have this as part of the private API for now?

Cheers,

Paul

Quoted 6 lines
include/uapi/linux/videodev2.h +0 -0
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
Signature · 1 line
2.20.1
Signature · 3 lines
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
#8 lore raw
Quoted 36 lines · Paul Kocialkowski

Hi,

Quoted 26 lines · Ezequiel Garcia

From: Pawel Osciak <[email protected]>

These controls are to be used with the new low-level decoder API for VP8 to provide additional parameters for the hardware that cannot parse the input stream.

[ezequiel: rebased]

---
As the H.264 interface is hopefully close to be merged, I'm sending the VP8 interface to start this discussion.

 drivers/media/v4l2-core/v4l2-ctrls.c |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/media/v4l2-ctrls.h           |   3 +
 include/media/vp8-ctrls.h            | 104 +++++++++++++++++++++++++++
 include/uapi/linux/videodev2.h       |   1 +
 5 files changed, 116 insertions(+)
 create mode 100644 include/media/vp8-ctrls.h
drivers/media/v4l2-core/v4l2-ctrls.c +0 -0
index 366200d31bc0..c77a56c3e2aa 100644

drivers/media/v4l2-core/v4l2-ctrls.c const char *v4l2_ctrl_get_name(u32 id) · line 872

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ctrls.c +1 -0
@@ -869,6 +869,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_HDR:			return "VP8 Frame Header";

Could we call this _FRAME_HEADER to match the control name?

Quoted 3 lines

I find it quite handy to have the ctrl definitions and associated structures named in sync.

Yeah, makes sense. And also get rid of similar names such as "SEGMNT".

Quoted 187 lines
Quoted 35 lines
drivers/media/v4l2-core/v4l2-ctrls.c +6 -0
 	/* HEVC controls */
 	case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:		return "HEVC I-Frame QP Value";
@@ -1323,6 +1324,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_HDR:
+		*type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1694,6 +1698,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_HDR:
 		return 0;
 
 	default:
@@ -2290,6 +2295,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 		break;
 	case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_decode_param);
+	case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
drivers/media/v4l2-core/v4l2-ioctl.c +0 -0
index c765c7c7c562..ea295aa9d0b6 100644

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1327

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ioctl.c +1 -0
@@ -1324,6 +1324,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;

I'm not familiar with VP8 at all, but are we guaranteed that only per-frame decoding mode is possible?

Quoted 144 lines
drivers/media/v4l2-core/v4l2-ioctl.c +0 -0
 		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 22b6d09c4764..183c7fc5d18d 100644
@@ -28,6 +28,7 @@
  */
 #include <media/mpeg2-ctrls.h>
 #include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
 
 /* forward references */
 struct file;
@@ -55,6 +56,7 @@ struct poll_table_struct;
  * @p_h264_scaling_matrix:	Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
  * @p_h264_slice_param:		Pointer to a struct v4l2_ctrl_h264_slice_param.
  * @p_h264_decode_param:	Pointer to a struct v4l2_ctrl_h264_decode_param.
+ * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -71,6 +73,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
 	struct v4l2_ctrl_h264_slice_param *p_h264_slice_param;
 	struct v4l2_ctrl_h264_decode_param *p_h264_decode_param;
+	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
 	void *p;
 };
 
include/media/vp8-ctrls.h new file +104 -0
new file mode 100644
index 000000000000..95b63a0cb239
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TODO: Make sure structs have no holes and are 4-byte aligned.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 260)
+
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR		0x220
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED              0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP           0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA  0x04
+
+struct v4l2_vp8_segment_header {
+	__u8 segment_feature_mode;
+	__s8 quant_update[4];
+	__s8 lf_update[4];
+	__u8 segment_probs[3];
+	__u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE	0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE	0x02
+struct v4l2_vp8_loopfilter_header {
+	__u16 type;
+	__u8 level;
+	__u8 sharpness_level;
+	__s8 ref_frm_delta_magnitude[4];
+	__s8 mb_mode_delta_magnitude[4];
+	__u16 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 dequant_factors[4][3][2];
+};
+
+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];
+};
+
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL		0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME		0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF	0x04
+struct v4l2_ctrl_vp8_frame_header {
+	/* 0: keyframe, 1: not a keyframe */
+	__u8 key_frame; // could be a flag?
+	__u8 version;
+
+	/* Populated also if not a key frame */
+	__u16 width;
+	__u16 height;
+	__u8 horizontal_scale;
+	__u8 vertical_scale;
+
+	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;
+
+	__u8 sign_bias_golden;
+	__u8 sign_bias_alternate;
+
+	__u8 prob_skip_false;
+	__u8 prob_intra;
+	__u8 prob_last;
+	__u8 prob_gf;
+
+	__u32 first_part_size;
+	__u32 first_part_offset; // this needed? it's always 3 + 7 * s->keyframe;
+	/*
+	 * Offset in bits of MB data in first partition,
+	 * i.e. bit offset starting from first_part_offset.
+	 */
+	__u32 macroblock_bit_offset;
+
+	__u8 num_dct_parts;
+	__u32 dct_part_sizes[8];
+
+	__u8 bool_dec_range;
+	__u8 bool_dec_value;
+	__u8 bool_dec_count;
+
+	__u64 last_frame_ts;
+	__u64 golden_frame_ts;
+	__u64 alt_frame_ts;
+
+	__u8 flags;
+};
+
+#endif
include/uapi/linux/videodev2.h +0 -1
index f6a484017208..a906bfc0c8f0 100644
--- a/include/uapi/linux/videodev2.h
Quoted 9 lines

include/uapi/linux/videodev2.h struct v4l2_pix_format { · line 667

Quoted 6 lines

+++ b/include/uapi/linux/videodev2.h

include/uapi/linux/videodev2.h +1 -0
@@ -664,6 +664,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */

Could we have this as part of the private API for now?

Sure, see the new version of this patch: https://patchwork.kernel.org/patch/10992145/
which is part of https://patchwork.kernel.org/cover/10992139/.

Thanks,
Eze