Message-ID: <5154683.x8GX3iSW2O@kista>
Date:   Mon, 09 Aug 2021 20:04:09 +0200
From:   Jernej Škrabec <jernej.skrabec@...il.com>
To:     linux-media@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        linux-staging@...ts.linux.dev,
        Andrzej Pietrasiewicz <andrzej.p@...labora.com>
Cc:     Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        Boris Brezillon <boris.brezillon@...labora.com>,
        Ezequiel Garcia <elezegarcia@...il.com>,
        Fabio Estevam <festevam@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Heiko Stuebner <heiko@...ech.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        NXP Linux Team <linux-imx@....com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>, kernel@...labora.com,
        Ezequiel Garcia <ezequiel@...labora.com>,
        Adrian Ratiu <adrian.ratiu@...labora.com>,
        Daniel Almeida <daniel.almeida@...labora.com>
Subject: Re: [PATCH v3 05/10] media: uapi: Add VP9 stateless decoder controls

Hi Andrzej!

Dne četrtek, 05. avgust 2021 ob 16:42:41 CEST je Andrzej Pietrasiewicz 
napisal(a):
> Add the VP9 stateless decoder controls plus the documentation that goes
> with it.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@...labora.com>
> Co-developed-by: Ezequiel Garcia <ezequiel@...labora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
> Co-developed-by: Daniel Almeida <daniel.almeida@...labora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
> ---
>  .../userspace-api/media/v4l/biblio.rst        |  10 +
>  .../media/v4l/ext-ctrls-codec-stateless.rst   | 545 ++++++++++++++++++
>  .../media/v4l/pixfmt-compressed.rst           |  15 +
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |   8 +
>  .../media/v4l/vidioc-queryctrl.rst            |  12 +
>  .../media/videodev2.h.rst.exceptions          |   2 +
>  drivers/media/v4l2-core/v4l2-ctrls-core.c     | 174 ++++++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   8 +
>  drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
>  include/media/v4l2-ctrls.h                    |   4 +
>  include/uapi/linux/v4l2-controls.h            | 433 ++++++++++++++
>  include/uapi/linux/videodev2.h                |   6 +
>  12 files changed, 1218 insertions(+)
> 

<snip>

> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-
controls.h
> index 5532b5f68493..8f8c3c1ad885 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -2010,6 +2010,439 @@ struct v4l2_ctrl_hdr10_mastering_display {
>  	__u32 min_display_mastering_luminance;
>  };
>  
> +/* Stateless VP9 controls */
> +
> +/**
> + * enum v4l2_vp9_loop_filter_flags - VP9 loop filter flags
> + *
> + * @V4L2_VP9_LOOP_FILTER_FLAG_DELTA_ENABLED: the filter level depends on
> + *	the mode and reference frame used to predict a block
> + * @V4L2_VP9_LOOP_FILTER_FLAG_DELTA_UPDATE: the bitstream contains 
additional
> + *	syntax elements that specify which mode and reference frame deltas 
are to be updated
> + *
> + * Those are the flags that may be passed to &v4l2_vp9_loop_filter.flags. See
> + * section '7.2.8 Loop filter semantics' of the VP9 specification for more
> + * details.
> + */
> +enum v4l2_vp9_loop_filter_flags {
> +	V4L2_VP9_LOOP_FILTER_FLAG_DELTA_ENABLED = 1 << 0,
> +	V4L2_VP9_LOOP_FILTER_FLAG_DELTA_UPDATE = 1 << 1,
> +};
> +
> +/**
> + * struct v4l2_vp9_loop_filter - VP9 loop filter parameters
> + *
> + * @ref_deltas: contains the adjustment needed for the filter level based on 
the chosen
> + *	reference frame. If this syntax element is not present in the 
bitstream, users
> + *	should pass its last value.
> + * @mode_deltas: contains the adjustment needed for the filter level based 
on the chosen mode.
> + *	If this syntax element is not present in the bitstream, users
> + *	should pass its last value.
> + * @level: indicates the loop filter strength
> + * @sharpness: indicates the sharpness level
> + * @flags: combination of V4L2_VP9_LOOP_FILTER_FLAG_* flags
> + * @reserved: padding field. Should be zeroed by applications.
> + *
> + * This structure contains all loop filter related parameters. See sections
> + * '7.2.8 Loop filter semantics' of the VP9 specification for more details.
> + */
> +struct v4l2_vp9_loop_filter {
> +	__s8 ref_deltas[4];
> +	__s8 mode_deltas[2];
> +	__u8 level;
> +	__u8 sharpness;
> +	__u8 flags;
> +	__u8 reserved[7];
> +};
> +
> +/**
> + * struct v4l2_vp9_quantization - VP9 quantization parameters
> + *
> + * @base_q_idx: indicates the base frame qindex
> + * @delta_q_y_dc: indicates the Y DC quantizer relative to base_q_idx
> + * @delta_q_uv_dc: indicates the UV DC quantizer relative to base_q_idx
> + * @delta_q_uv_ac: indicates the UV AC quantizer relative to base_q_idx
> + * @reserved: padding field. Should be zeroed by applications.
> + *
> + * Encodes the quantization parameters. See section '7.2.9 Quantization 
params
> + * syntax' of the VP9 specification for more details.
> + */
> +struct v4l2_vp9_quantization {
> +	__u8 base_q_idx;
> +	__s8 delta_q_y_dc;
> +	__s8 delta_q_uv_dc;
> +	__s8 delta_q_uv_ac;
> +	__u8 reserved[4];
> +};
> +
> +/**
> + * enum v4l2_vp9_segmentation_flags - VP9 segmentation flags
> + *
> + * @V4L2_VP9_SEGMENTATION_FLAG_ENABLED: indicates that this frame makes use 
of
> + *	the segmentation tool
> + * @V4L2_VP9_SEGMENTATION_FLAG_UPDATE_MAP: indicates that the segmentation 
map
> + *	should be updated during the decoding of this frame
> + * @V4L2_VP9_SEGMENTATION_FLAG_TEMPORAL_UPDATE: indicates that the updates 
to
> + *	the segmentation map are coded relative to the existing 
segmentation map
> + * @V4L2_VP9_SEGMENTATION_FLAG_UPDATE_DATA: indicates that new parameters 
are
> + *	about to be specified for each segment
> + * @V4L2_VP9_SEGMENTATION_FLAG_ABS_OR_DELTA_UPDATE: indicates that the
> + *	segmentation parameters represent the actual values to be used
> + *
> + * Those are the flags that may be passed to &v4l2_vp9_segmentation.flags. 
See
> + * section '7.2.10 Segmentation params syntax' of the VP9 specification for
> + * more details.
> + */
> +enum v4l2_vp9_segmentation_flags {
> +	V4L2_VP9_SEGMENTATION_FLAG_ENABLED = 1 << 0,
> +	V4L2_VP9_SEGMENTATION_FLAG_UPDATE_MAP = 1 << 1,
> +	V4L2_VP9_SEGMENTATION_FLAG_TEMPORAL_UPDATE = 1 << 2,
> +	V4L2_VP9_SEGMENTATION_FLAG_UPDATE_DATA = 1 << 3,
> +	V4L2_VP9_SEGMENTATION_FLAG_ABS_OR_DELTA_UPDATE = 1 << 4,
> +};
> +
> +#define V4L2_VP9_SEGMENT_FEATURE_ENABLED(id)	(1 << (id))
> +#define V4L2_VP9_SEGMENT_FEATURE_ENABLED_MASK	0xf
> +
> +/**
> + * enum v4l2_vp9_segment_feature - VP9 segment feature IDs
> + *
> + * @V4L2_VP9_SEG_LVL_ALT_Q: quantizer segment feature
> + * @V4L2_VP9_SEG_LVL_ALT_L: loop filter segment feature
> + * @V4L2_VP9_SEG_LVL_REF_FRAME reference frame segment feature
> + * @V4L2_VP9_SEG_LVL_SKIP: skip segment feature
> + * @V4L2_VP9_SEG_LVV_MAX: number of segment features
> + *
> + * Segment feature IDs. See section '7.2.10 Segmentation params syntax' of 
the
> + * VP9 specification for more details.
> + */
> +enum v4l2_vp9_segment_feature {
> +	V4L2_VP9_SEG_LVL_ALT_Q,
> +	V4L2_VP9_SEG_LVL_ALT_L,
> +	V4L2_VP9_SEG_LVL_REF_FRAME,
> +	V4L2_VP9_SEG_LVL_SKIP,
> +	V4L2_VP9_SEG_LVL_MAX,
> +};

What is the reason not to assign values like 1 << 0, 1 << 1, etc. and instead 
provide separate macro V4L2_VP9_SEGMENT_FEATURE_ENABLED()? This is a departure 
from current practice.

> +
> +/**
> + * struct v4l2_vp9_segmentation - VP9 segmentation parameters
> + *
> + * @feature_data: data attached to each feature. Data entry is only valid 
if the feature is enabled
> + * @feature_enabled: bitmask defining which features are enabled in each 
segment
> + * @tree_probs: specifies the probability values to be used when
> + *	decoding a Segment-ID. See '5.15. Segmentation map' section of the 
VP9 specification
> + *	for more details.
> + * @pred_probs: specifies the probability values to be used when decoding a
> + *	Predicted-Segment-ID. See '6.4.14. Get segment id syntax' section 
of :ref:`vp9`
> + *	for more details.
> + * @flags: combination of V4L2_VP9_SEGMENTATION_FLAG_* flags
> + * @reserved: padding field. Should be zeroed by applications.
> + *
> + * Encodes the quantization parameters. See section '7.2.10 Segmentation 
params syntax' of
> + * the VP9 specification for more details.
> + */
> +struct v4l2_vp9_segmentation {
> +	__s16 feature_data[8][4];
> +	__u8 feature_enabled[8];
> +	__u8 tree_probs[7];
> +	__u8 pred_probs[3];
> +	__u8 flags;
> +	__u8 reserved[5];
> +};
> +
> +/**
> + * enum v4l2_vp9_intra_prediction_mode - VP9 Intra prediction modes
> + *
> + * @V4L2_VP9_INTRA_PRED_MODE_DC_PRED: DC intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_V_PRED: vertical intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_H_PRED: horizontal intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D45_PRED: D45 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D135_PRED: D135 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D117_PRED: D117 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D153_PRED: D153 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D207_PRED: D207 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_D63_PRED: D63 intra prediction
> + * @V4L2_VP9_INTRA_PRED_MODE_TM_PRED: True Motion intra prediction
> + *
> + * See section '7.4.5 Intra frame mode info semantics' for more details.
> + */
> +enum v4l2_vp9_intra_prediction_mode {
> +	V4L2_VP9_INTRA_PRED_MODE_DC_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_V_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_H_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D45_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D135_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D117_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D153_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D207_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_D63_PRED,
> +	V4L2_VP9_INTRA_PRED_MODE_TM_PRED,
> +};

I dislike using enums without assigning values. I see that historically (at 
the beginning of this file), values were always assigned. However, if 
maintainers agree with you, that's fine by me.

> +
> +/**
> + * enum v4l2_vp9_reset_frame_context - Valid values for
> + *			&v4l2_ctrl_vp9_frame->reset_frame_context
> + *
> + * @V4L2_VP9_RESET_FRAME_CTX_NONE: don't reset any frame context
> + * @V4L2_VP9_RESET_FRAME_CTX_SPEC: reset the frame context pointed by
> + *	&v4l2_ctrl_vp9_frame.frame_context_idx
> + * @V4L2_VP9_RESET_FRAME_CTX_ALL: reset all frame contexts
> + *
> + * See section '7.2 Uncompressed header semantics' of the VP9 specification 
for more details.
> + */
> +enum v4l2_vp9_reset_frame_context {
> +	V4L2_VP9_RESET_FRAME_CTX_NONE,
> +	V4L2_VP9_RESET_FRAME_CTX_SPEC,
> +	V4L2_VP9_RESET_FRAME_CTX_ALL,
> +};
> +
> +/**
> + * enum v4l2_vp9_interpolation_filter - VP9 interpolation filter types
> + *
> + * @V4L2_VP9_INTERP_FILTER_EIGHTTAP: eight tap filter
> + * @V4L2_VP9_INTERP_FILTER_EIGHTTAP_SMOOTH: eight tap smooth filter
> + * @V4L2_VP9_INTERP_FILTER_EIGHTTAP_SHARP: eight tap sharp filter
> + * @V4L2_VP9_INTERP_FILTER_BILINEAR: bilinear filter
> + * @V4L2_VP9_INTERP_FILTER_SWITCHABLE: filter selection is signaled at the 
block level
> + *
> + * See section '7.2.7 Interpolation filter semantics' of the VP9 
specification
> + * for more details.
> + */
> +enum v4l2_vp9_interpolation_filter {
> +	V4L2_VP9_INTERP_FILTER_EIGHTTAP,
> +	V4L2_VP9_INTERP_FILTER_EIGHTTAP_SMOOTH,
> +	V4L2_VP9_INTERP_FILTER_EIGHTTAP_SHARP,
> +	V4L2_VP9_INTERP_FILTER_BILINEAR,
> +	V4L2_VP9_INTERP_FILTER_SWITCHABLE,
> +};
> +
> +/**
> + * enum v4l2_vp9_reference_mode - VP9 reference modes
> + *
> + * @V4L2_VP9_REFERENCE_MODE_SINGLE_REFERENCE: indicates that all the inter 
blocks use only a
> + *	single reference frame to generate motion compensated prediction
> + * @V4L2_VP9_REFERENCE_MODE_COMPOUND_REFERENCE: requires all the inter 
blocks to use compound mode.
> + *	Single reference frame prediction is not allowed
> + * @V4L2_VP9_REFERENCE_MODE_SELECT: allows each individual inter block to 
select between single
> + *	and compound prediction modes
> + *
> + * See section '7.3.6 Frame reference mode semantics' of the VP9 
specification
> + * for more details.
> + */
> +enum v4l2_vp9_reference_mode {
> +	V4L2_VP9_REFERENCE_MODE_SINGLE_REFERENCE,
> +	V4L2_VP9_REFERENCE_MODE_COMPOUND_REFERENCE,
> +	V4L2_VP9_REFERENCE_MODE_SELECT,
> +};
> +
> +/**
> + * enum v4l2_vp9_tx_mode - VP9 TX modes
> + *
> + * @V4L2_VP9_TX_MODE_ONLY_4X4: transform size is 4x4
> + * @V4L2_VP9_TX_MODE_ALLOW_8X8: transform size can be up to 8x8
> + * @V4L2_VP9_TX_MODE_ALLOW_16X16: transform size can be up to 16x16
> + * @V4L2_VP9_TX_MODE_ALLOW_32X32: transform size can be up to 32x32
> + * @V4L2_VP9_TX_MODE_SELECT: bitstream contains the transform size for each 
block
> + *
> + * See section '7.3.1 Tx mode semantics' of the VP9 specification for more 
details.
> + */
> +enum v4l2_vp9_tx_mode {
> +	V4L2_VP9_TX_MODE_ONLY_4X4,
> +	V4L2_VP9_TX_MODE_ALLOW_8X8,
> +	V4L2_VP9_TX_MODE_ALLOW_16X16,
> +	V4L2_VP9_TX_MODE_ALLOW_32X32,
> +	V4L2_VP9_TX_MODE_SELECT,
> +};
> +
> +/**
> + * enum v4l2_vp9_frame_flags - VP9 frame flags
> + * @V4L2_VP9_FRAME_FLAG_KEY_FRAME: the frame is a key frame
> + * @V4L2_VP9_FRAME_FLAG_SHOW_FRAME: the frame should be displayed
> + * @V4L2_VP9_FRAME_FLAG_ERROR_RESILIENT: the decoding should be error 
resilient
> + * @V4L2_VP9_FRAME_FLAG_INTRA_ONLY: the frame does not reference other 
frames
> + * @V4L2_VP9_FRAME_FLAG_ALLOW_HIGH_PREC_MV: the frame can use high 
precision motion vectors
> + * @V4L2_VP9_FRAME_FLAG_REFRESH_FRAME_CTX: frame context should be updated 
after decoding
> + * @V4L2_VP9_FRAME_FLAG_PARALLEL_DEC_MODE: parallel decoding is used
> + * @V4L2_VP9_FRAME_FLAG_X_SUBSAMPLING: vertical subsampling is enabled
> + * @V4L2_VP9_FRAME_FLAG_Y_SUBSAMPLING: horizontal subsampling is enabled
> + * @V4L2_VP9_FRAME_FLAG_COLOR_RANGE_FULL_SWING: full UV range is used
> + *
> + * Check the VP9 specification for more details.
> + */
> +enum v4l2_vp9_frame_flags {
> +	V4L2_VP9_FRAME_FLAG_KEY_FRAME = 1 << 0,
> +	V4L2_VP9_FRAME_FLAG_SHOW_FRAME = 1 << 1,
> +	V4L2_VP9_FRAME_FLAG_ERROR_RESILIENT = 1 << 2,
> +	V4L2_VP9_FRAME_FLAG_INTRA_ONLY = 1 << 3,
> +	V4L2_VP9_FRAME_FLAG_ALLOW_HIGH_PREC_MV = 1 << 4,
> +	V4L2_VP9_FRAME_FLAG_REFRESH_FRAME_CTX = 1 << 5,
> +	V4L2_VP9_FRAME_FLAG_PARALLEL_DEC_MODE = 1 << 6,
> +	V4L2_VP9_FRAME_FLAG_X_SUBSAMPLING = 1 << 7,
> +	V4L2_VP9_FRAME_FLAG_Y_SUBSAMPLING = 1 << 8,
> +	V4L2_VP9_FRAME_FLAG_COLOR_RANGE_FULL_SWING = 1 << 9,
> +};
> +
> +/**
> + * enum v4l2_vp9_ref_frame_sign_bias - Whether ref_frame_sign_bias is 1 for 
a given reference frame
> + * @V4L2_VP9_SIGN_BIAS_LAST: ref_frame_sign_bias is 1 for the "last" 
reference frame
> + * @V4L2_VP9_SIGN_BIAS_GOLDEN: ref_frame_sign_bias is 1 for the "golden" 
reference frame
> + * @V4L2_VP9_SIGN_BIAS_ALT: ref_frame_sign_bias is 1 for the "alt" 
reference frame
> + */
> +
> +enum v4l2_vp9_ref_frame_sign_bias {
> +	V4L2_VP9_SIGN_BIAS_LAST,
> +	V4L2_VP9_SIGN_BIAS_GOLDEN,
> +	V4L2_VP9_SIGN_BIAS_ALT,
> +};

As I mentioned before, you probably want to assign values here, since values 
are used as bitmap.

> +
> +#define V4L2_VP9_PROFILE_MAX		3
> +
> +#define V4L2_CID_STATELESS_VP9_FRAME	(V4L2_CID_CODEC_STATELESS_BASE + 
300)
> +/**
> + * struct v4l2_ctrl_vp9_frame - VP9 frame decoding control
> + *
> + * @lf: loop filter parameters. See &v4l2_vp9_loop_filter for more details
> + * @quant: quantization parameters. See &v4l2_vp9_quantization for more 
details
> + * @seg: segmentation parameters. See &v4l2_vp9_segmentation for more 
details
> + * @flags: combination of V4L2_VP9_FRAME_FLAG_* flags
> + * @compressed_header_size: compressed header size in bytes
> + * @uncompressed_header_size: uncompressed header size in bytes
> + * @frame_width_minus_1: add 1 to it and you'll get the frame width 
expressed in pixels
> + * @frame_height_minus_1: add 1 to it and you'll get the frame height 
expressed in pixels
> + * @render_width_minus_1: add 1 to it and you'll get the expected render 
width expressed in
> + *	pixels. This is not used during the decoding process but might be 
used by HW scalers
> + *	to prepare a frame that's ready for scanout
> + * @render_height_minus_1: add 1 to it and you'll get the expected render 
height expressed in
> + *	pixels. This is not used during the decoding process but might be 
used by HW scalers
> + *	to prepare a frame that's ready for scanout
> + * @last_frame_ts: "last" reference buffer timestamp.
> + * The timestamp refers to the timestamp field in struct v4l2_buffer.
> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64.
> + * @golden_frame_ts: "golden" reference buffer timestamp.
> + * The timestamp refers to the timestamp field in struct v4l2_buffer.
> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64.
> + * @alt_frame_ts: "alt" reference buffer timestamp.
> + * The timestamp refers to the timestamp field in struct v4l2_buffer.
> + * Use v4l2_timeval_to_ns() to convert the struct timeval to a __u64.
> + * @ref_frame_sign_bias: a bitfield specifying whether the sign bias is set 
for a given
> + *	reference frame. See &v4l2_vp9_ref_frame_sign_bias for more 
details.
> + * @reset_frame_context: specifies whether the frame context should be reset 
to default values.
> + *	See &v4l2_vp9_reset_frame_context for more details
> + * @frame_context_idx: frame context that should be used/updated
> + * @profile: VP9 profile. Can be 0, 1, 2 or 3
> + * @bit_depth: bits per components. Can be 8, 10 or 12. Note that not all 
profiles support
> + *	10 and/or 12 bits depths
> + * @interpolation_filter: specifies the filter selection used for performing 
inter prediction.
> + *	See &v4l2_vp9_interpolation_filter for more details
> + * @tile_cols_log2: specifies the base 2 logarithm of the width of each tile 
(where the width
> + *	is measured in units of 8x8 blocks). Shall be less than or equal 
to 6
> + * @tile_rows_log2: specifies the base 2 logarithm of the height of each 
tile (where the height
> + *	is measured in units of 8x8 blocks)
> + * @tx_mode: specifies the TX mode. See &v4l2_vp9_tx_mode for more details
> + * @reference_mode: specifies the type of inter prediction to be used. See
> + *	&v4l2_vp9_reference_mode for more details
> + * @reserved: padding field. Should be zeroed by applications.
> + */
> +struct v4l2_ctrl_vp9_frame {
> +	struct v4l2_vp9_loop_filter lf;
> +	struct v4l2_vp9_quantization quant;
> +	struct v4l2_vp9_segmentation seg;
> +	__u32 flags;
> +	__u16 compressed_header_size;
> +	__u16 uncompressed_header_size;
> +	__u16 frame_width_minus_1;
> +	__u16 frame_height_minus_1;
> +	__u16 render_width_minus_1;
> +	__u16 render_height_minus_1;
> +	__u64 last_frame_ts;
> +	__u64 golden_frame_ts;
> +	__u64 alt_frame_ts;
> +	__u8 ref_frame_sign_bias;
> +	__u8 reset_frame_context;
> +	__u8 frame_context_idx;
> +	__u8 profile;
> +	__u8 bit_depth;
> +	__u8 interpolation_filter;
> +	__u8 tile_cols_log2;
> +	__u8 tile_rows_log2;
> +	__u8 tx_mode;

AFAICS, TX mode is part of compressed header in chapter 6.3.1. Any reason it's 
here and not in  v4l2_ctrl_vp9_compressed_hdr_probs?

> +	__u8 reference_mode;
> +	__u8 reserved[6];
> +};
> +
> +#define V4L2_VP9_NUM_FRAME_CTX	4
> +
> +/**
> + * struct v4l2_vp9_mv_compressed_hdr_probs - VP9 Motion vector probability 
updates
> + * @joint: motion vector joint probability updates
> + * @sign: motion vector sign probability updates
> + * @classes: motion vector class probability updates
> + * @class0_bit: motion vector class0 bit probability updates
> + * @bits: motion vector bits probability updates
> + * @class0_fr: motion vector class0 fractional bit probability updates
> + * @fr: motion vector fractional bit probability updates
> + * @class0_hp: motion vector class0 high precision fractional bit 
probability updates
> + * @hp: motion vector high precision fractional bit probability updates
> + *
> + * This structure contains new values of motion vector probabilities.
> + * A value of zero in an array element means there is no update of the 
relevant probability.
> + * See `struct v4l2_vp9_prob_updates` for details.
> + */
> +struct v4l2_vp9_mv_compressed_hdr_probs {
> +	__u8 joint[3];
> +	__u8 sign[2];
> +	__u8 classes[2][10];
> +	__u8 class0_bit[2];
> +	__u8 bits[2][10];
> +	__u8 class0_fr[2][2][3];
> +	__u8 fr[2][3];
> +	__u8 class0_hp[2];
> +	__u8 hp[2];
> +};
> +
> +#define V4L2_CID_STATELESS_VP9_COMPRESSED_HDR_PROBS	
(V4L2_CID_CODEC_STATELESS_BASE + 301)
> +/**
> + * struct v4l2_ctrl_vp9_compressed_hdr_probs - VP9 probability updates 
control
> + * @tx8: TX 8x8 probability updates
> + * @tx16: TX 16x16 probability updates
> + * @tx32: TX 32x32 probability updates
> + * @coef: coefficient probability updates
> + * @skip: skip probability updates
> + * @inter_mode: inter mode probability updates
> + * @interp_filter: interpolation filter probability updates
> + * @is_inter: is inter-block probability updates
> + * @comp_mode: compound prediction mode probability updates
> + * @single_ref: single ref probability updates
> + * @comp_ref: compound ref probability updates
> + * @y_mode: Y prediction mode probability updates
> + * @uv_mode: UV prediction mode probability updates
> + * @partition: partition probability updates
> + * @mv: motion vector probability updates
> + *
> + * This structure holds the probabilities update as parsed in the 
compressed
> + * header (Spec 6.3). These values represent the value of probability 
update after
> + * being translated with inv_map_table[] (see 6.3.5). A value of zero in an 
array element
> + * means that there is no update of the relevant probability.
> + */
> +struct v4l2_ctrl_vp9_compressed_hdr_probs {
> +	__u8 tx8[2][1];
> +	__u8 tx16[2][2];
> +	__u8 tx32[2][3];
> +	__u8 coef[4][2][2][6][6][3];
> +	__u8 skip[3];
> +	__u8 inter_mode[7][3];
> +	__u8 interp_filter[4][2];
> +	__u8 is_inter[4];
> +	__u8 comp_mode[5];
> +	__u8 single_ref[5][2];
> +	__u8 comp_ref[5];
> +	__u8 y_mode[4][9];
> +	__u8 uv_mode[10][9];
> +	__u8 partition[16][3];
> +
> +	struct v4l2_vp9_mv_compressed_hdr_probs mv;

v4l2_ctrl_vp9_compressed_hdr_probs and v4l2_vp9_mv_compressed_hdr_probs are so 
confusingly similar, it's easy to mess up quickly. Can we find more distinct 
names?

Note, I just skimmed this patch, maybe I'll have some more questions later. 
Great work!

Best regards,
Jernej

> +};
> +
>  /* MPEG-compression definitions kept for backwards compatibility */
>  #ifndef __KERNEL__
>  #define V4L2_CTRL_CLASS_MPEG            V4L2_CTRL_CLASS_CODEC
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 58392dcd3bf5..4abe4362519e 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -703,6 +703,7 @@ struct v4l2_pix_format {
>  #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 
frame */
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
> +#define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed 
frame */
>  #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) */
>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* 
Stateless FWHT (vicodec) */
> @@ -1755,6 +1756,8 @@ struct v4l2_ext_control {
>  		struct v4l2_ctrl_mpeg2_sequence __user 
*p_mpeg2_sequence;
>  		struct v4l2_ctrl_mpeg2_picture __user *p_mpeg2_picture;
>  		struct v4l2_ctrl_mpeg2_quantisation __user 
*p_mpeg2_quantisation;
> +		struct v4l2_ctrl_vp9_compressed_hdr_probs __user 
*p_vp9_compressed_hdr_probs;
> +		struct v4l2_ctrl_vp9_frame __user *p_vp9_frame;
>  		void __user *ptr;
>  	};
>  } __attribute__ ((packed));
> @@ -1819,6 +1822,9 @@ enum v4l2_ctrl_type {
>  	V4L2_CTRL_TYPE_MPEG2_QUANTISATION   = 0x0250,
>  	V4L2_CTRL_TYPE_MPEG2_SEQUENCE       = 0x0251,
>  	V4L2_CTRL_TYPE_MPEG2_PICTURE        = 0x0252,
> +
> +	V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR_PROBS	= 0x0260,
> +	V4L2_CTRL_TYPE_VP9_FRAME		= 0x0261,
>  };
>  
>  /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> -- 
> 2.17.1
> 
> 

Powered by blists - more mailing lists