lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 04 Jul 2019 10:00:33 -0300
From:   Ezequiel Garcia <ezequiel@...labora.com>
To:     Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        linux-media@...r.kernel.org, Hans Verkuil <hans.verkuil@...co.com>
Cc:     kernel@...labora.com, Tomasz Figa <tfiga@...omium.org>,
        linux-rockchip@...ts.infradead.org,
        Heiko Stuebner <heiko@...ech.de>,
        Jonas Karlman <jonas@...boo.se>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Boris Brezillon <boris.brezillon@...labora.com>,
        Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        Alexandre Courbot <acourbot@...omium.org>,
        fbuergisser@...omium.org, linux-kernel@...r.kernel.org,
        Pawel Osciak <posciak@...omium.org>
Subject: Re: [PATCH v2 1/2] media: uapi: Add VP8 stateless decoder API

On Wed, 2019-07-03 at 13:39 -0400, Nicolas Dufresne wrote:
> Le mardi 02 juillet 2019 à 14:00 -0300, Ezequiel Garcia a écrit :
> > From: Pawel Osciak <posciak@...omium.org>
> > 
> > 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 <posciak@...omium.org>
> > Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
> > --
> > 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
> > 
> > diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> > index 8f4eb8823d82..ad2ff258afa8 100644
> > --- a/Documentation/media/uapi/v4l/biblio.rst
> > +++ b/Documentation/media/uapi/v4l/biblio.rst
> > @@ -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.
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > index d6ea2ffd65c5..aef335f175cd 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > @@ -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.
> 
> nit: I'd remove the "it"
> 

OK.

> > +
> > +.. 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!
> 
> nit: "NOT IN THE SPEC" -> "This value is not explicit in the frame
> header."
> 

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

> 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".


> > +    * - __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
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > index 4b701fc7653e..f52a7b67023d 100644
> > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > @@ -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
> 
> nit: Same, would drop the "it".
> 

OK.

> > +	   is expected to change.
> > +
> >      * .. _V4L2-PIX-FMT-VP9:
> >  
> >        - ``V4L2_PIX_FMT_VP9``
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 371537dd8cd3..7f94c431d94e 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -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);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index b1f4b991dba6..436a13204921 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -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 */
> > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > index b4433483af23..6e9dc9c44bb1 100644
> > --- a/include/media/v4l2-ctrls.h
> > +++ b/include/media/v4l2-ctrls.h
> > @@ -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;
> >  };
> >  
> > diff --git a/include/media/vp8-ctrls.h b/include/media/vp8-ctrls.h
> > new file mode 100644
> > index 000000000000..636442dc18aa
> > --- /dev/null
> > +++ b/include/media/vp8-ctrls.h
> > @@ -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
> 
> 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.
> 

I'll submit a v3, addressing yours and Philipp's feedback.

> (even if you already got that from Tomasz)
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>

Thanks a lot for the review,
Ezequiel

Powered by blists - more mailing lists