| 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 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Mar 2019 15:12:18 +0900
From: Tomasz Figa <tfiga@...omium.org>
To: Maxime Ripard <maxime.ripard@...tlin.com>
Cc: Hans Verkuil <hans.verkuil@...co.com>,
Alexandre Courbot <acourbot@...omium.org>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Pawel Osciak <posciak@...omium.org>,
Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
Chen-Yu Tsai <wens@...e.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
Roedel <joro@...tes.org>," <linux-arm-kernel@...ts.infradead.org>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
jenskuske@...il.com, Jernej Skrabec <jernej.skrabec@...il.com>,
Jonas Karlman <jonas@...boo.se>,
Ezequiel Garcia <ezequiel@...labora.com>,
linux-sunxi@...glegroups.com,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH v5 1/2] media: uapi: Add H264 low-level decoder API
compound controls.
Hi Maxime,
On Thu, Mar 7, 2019 at 10:20 PM Maxime Ripard <maxime.ripard@...tlin.com> wrote:
>
> From: Pawel Osciak <posciak@...omium.org>
>
> Stateless video codecs will require both the H264 metadata and slices in
> order to be able to decode frames.
>
> This introduces the definitions for a new pixel format for H264 slices that
> have been parsed, as well as the structures used to pass the metadata from
> the userspace to the kernel.
>
> Co-Developped-by: Maxime Ripard <maxime.ripard@...tlin.com>
> Signed-off-by: Pawel Osciak <posciak@...omium.org>
> Signed-off-by: Guenter Roeck <groeck@...omium.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@...tlin.com>
> ---
> Documentation/media/uapi/v4l/biblio.rst | 9 +-
> Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 549 ++++++++++++++-
> Documentation/media/uapi/v4l/pixfmt-compressed.rst | 19 +-
> Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +-
> Documentation/media/videodev2.h.rst.exceptions | 5 +-
> drivers/media/v4l2-core/v4l2-ctrls.c | 42 +-
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +-
> include/media/h264-ctrls.h | 190 +++++-
> include/media/v4l2-ctrls.h | 13 +-
> include/uapi/linux/videodev2.h | 1 +-
> 10 files changed, 858 insertions(+), 1 deletion(-)
> create mode 100644 include/media/h264-ctrls.h
>
Thanks for addressing my comments! Few more inline. (Sorry, missed
some previously. :()
> diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> index ec33768c055e..3fc3f7ff338a 100644
> --- a/Documentation/media/uapi/v4l/biblio.rst
> +++ b/Documentation/media/uapi/v4l/biblio.rst
> @@ -122,6 +122,15 @@ ITU BT.1119
>
> :author: International Telecommunication Union (http://www.itu.ch)
>
> +.. _h264:
> +
> +ITU H.264
> +=========
> +
> +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services"
> +
> +:author: International Telecommunication Union (http://www.itu.ch)
> +
> .. _jfif:
>
> JFIF
> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> index 54b3797b67dd..685942f0300e 100644
> --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> @@ -1343,6 +1343,555 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> - Layer number
>
>
> +.. _v4l2-mpeg-h264:
> +
> +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)``
> + Specifies the sequence parameter set (as extracted from the
> + bitstream) for the associated H264 slice data. This includes the
> + necessary parameters for configuring a stateless hardware decoding
> + pipeline for H264. The bitstream parameters are defined according
> + to :ref:`h264`. Unless there's a specific comment, refer to the
> + specification for the documentation of these fields, section 7.4.2.1.1
> + "Sequence Parameter Set Data Semantics".
I don't see this section being added by this patch. Where does it come from?
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_ctrl_h264_sps
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_h264_sps
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u8
> + - ``profile_idc``
> + -
> + * - __u8
> + - ``constraint_set_flags``
> + - See :ref:`Sequence Parameter Set Constraints Set Flags <h264_sps_constraints_set_flags>`
> + * - __u8
> + - ``level_idc``
> + -
> + * - __u8
> + - ``seq_parameter_set_id``
> + -
> + * - __u8
> + - ``chroma_format_idc``
> + -
> + * - __u8
> + - ``bit_depth_luma_minus8``
> + -
> + * - __u8
> + - ``bit_depth_chroma_minus8``
> + -
> + * - __u8
> + - ``log2_max_frame_num_minus4``
> + -
> + * - __u8
> + - ``pic_order_cnt_type``
> + -
> + * - __u8
> + - ``log2_max_pic_order_cnt_lsb_minus4``
> + -
> + * - __u8
> + - ``max_num_ref_frames``
> + -
> + * - __u8
> + - ``num_ref_frames_in_pic_order_cnt_cycle``
> + -
> + * - __s32
> + - ``offset_for_ref_frame[255]``
> + -
> + * - __s32
> + - ``offset_for_non_ref_pic``
> + -
> + * - __s32
> + - ``offset_for_top_to_bottom_field``
> + -
> + * - __u16
> + - ``pic_width_in_mbs_minus1``
> + -
> + * - __u16
> + - ``pic_height_in_map_units_minus1``
> + -
> + * - __u32
> + - ``flags``
> + - See :ref:`Sequence Parameter Set Flags <h264_sps_flags>`
> +
> +.. _h264_sps_constraints_set_flags:
> +
> +``Sequence Parameter Set Constraints Set Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET0_FLAG``
> + - 0x00000001
> + -
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET1_FLAG``
> + - 0x00000002
> + -
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET2_FLAG``
> + - 0x00000004
> + -
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET3_FLAG``
> + - 0x00000008
> + -
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET4_FLAG``
> + - 0x00000010
> + -
> + * - ``V4L2_H264_SPS_CONSTRAINT_SET5_FLAG``
> + - 0x00000020
> + -
> +
> +.. _h264_sps_flags:
> +
> +``Sequence Parameter Set Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_H264_SPS_FLAG_SEPARATE_COLOUR_PLANE``
> + - 0x00000001
> + -
> + * - ``V4L2_H264_SPS_FLAG_QPPRIME_Y_ZERO_TRANSFORM_BYPASS``
> + - 0x00000002
> + -
> + * - ``V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO``
> + - 0x00000004
> + -
> + * - ``V4L2_H264_SPS_FLAG_GAPS_IN_FRAME_NUM_VALUE_ALLOWED``
> + - 0x00000008
> + -
> + * - ``V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY``
> + - 0x00000010
> + -
> + * - ``V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD``
> + - 0x00000020
> + -
> + * - ``V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE``
> + - 0x00000040
> + -
> +
> +``V4L2_CID_MPEG_VIDEO_H264_PPS (struct)``
> + Specifies the picture parameter set (as extracted from the
> + bitstream) for the associated H264 slice data. This includes the
> + necessary parameters for configuring a stateless hardware decoding
> + pipeline for H264. The bitstream parameters are defined according
> + to :ref:`h264`. Unless there's a specific comment, refer to the
> + specification for the documentation of these fields, section 7.4.2.2
> + "Picture Parameter Set RBSP Semantics".
Ditto.
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_ctrl_h264_pps
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_h264_pps
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u8
> + - ``pic_parameter_set_id``
> + -
> + * - __u8
> + - ``seq_parameter_set_id``
> + -
> + * - __u8
> + - ``num_slice_groups_minus1``
> + -
> + * - __u8
> + - ``num_ref_idx_l0_default_active_minus1``
> + -
> + * - __u8
> + - ``num_ref_idx_l1_default_active_minus1``
> + -
> + * - __u8
> + - ``weighted_bipred_idc``
> + -
> + * - __s8
> + - ``pic_init_qp_minus26``
> + -
> + * - __s8
> + - ``pic_init_qs_minus26``
> + -
> + * - __s8
> + - ``chroma_qp_index_offset``
> + -
> + * - __s8
> + - ``second_chroma_qp_index_offset``
> + -
> + * - __u16
> + - ``flags``
> + - See :ref:`Picture Parameter Set Flags <h264_pps_flags>`
> +
> +.. _h264_pps_flags:
> +
> +``Picture Parameter Set Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE``
> + - 0x00000001
> + -
> + * - ``V4L2_H264_PPS_FLAG_BOTTOM_FIELD_PIC_ORDER_IN_FRAME_PRESENT``
> + - 0x00000002
> + -
> + * - ``V4L2_H264_PPS_FLAG_WEIGHTED_PRED``
> + - 0x00000004
> + -
> + * - ``V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT``
> + - 0x00000008
> + -
> + * - ``V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED``
> + - 0x00000010
> + -
> + * - ``V4L2_H264_PPS_FLAG_REDUNDANT_PIC_CNT_PRESENT``
> + - 0x00000020
> + -
> + * - ``V4L2_H264_PPS_FLAG_TRANSFORM_8X8_MODE``
> + - 0x00000040
> + -
> + * - ``V4L2_H264_PPS_FLAG_PIC_SCALING_MATRIX_PRESENT``
> + - 0x00000080
> + -
> +
> +``V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX (struct)``
> + Specifies the scaling matrix (as extracted from the bitstream) for
> + the associated H264 slice data. The bitstream parameters are
> + defined according to :ref:`h264`. Unless there's a specific
> + comment, refer to the specification for the documentation of these
> + fields, section 7.4.2.1.1.1 "Scaling List Semantics".
Ditto.
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_ctrl_h264_scaling_matrix
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_h264_scaling_matrix
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u8
> + - ``scaling_list_4x4[6][16]``
> + -
> + * - __u8
> + - ``scaling_list_8x8[6][64]``
> + -
> +
> +``V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS (struct)``
> + Specifies the slice parameters (as extracted from the bitstream)
> + for the associated H264 slice data. This includes the necessary
> + parameters for configuring a stateless hardware decoding pipeline
> + for H264. The bitstream parameters are defined according to
> + :ref:`h264`. Unless there's a specific comment, refer to the
> + specification for the documentation of these fields, section 7.4.3
> + "Slice Header Semantics".
Ditto.
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API
> + and it is expected to change.
> +
> + This structure is expected to be passed as an array, with one
> + entry for each slice included in the bitstream buffer.
> +
> +.. c:type:: v4l2_ctrl_h264_slice_param
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_h264_slice_param
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u32
> + - ``size``
> + -
> + * - __u32
> + - ``header_bit_size``
> + -
> + * - __u16
> + - ``first_mb_in_slice``
> + -
> + * - __u8
> + - ``slice_type``
> + -
> + * - __u8
> + - ``pic_parameter_set_id``
> + -
> + * - __u8
> + - ``colour_plane_id``
> + -
> + * - __u8
> + - ``redundant_pic_cnt``
> + -
> + * - __u16
> + - ``frame_num``
> + -
> + * - __u16
> + - ``idr_pic_id``
> + -
> + * - __u16
> + - ``pic_order_cnt_lsb``
> + -
> + * - __s32
> + - ``delta_pic_order_cnt_bottom``
> + -
> + * - __s32
> + - ``delta_pic_order_cnt0``
> + -
> + * - __s32
> + - ``delta_pic_order_cnt1``
> + -
> + * - struct :c:type:`v4l2_h264_pred_weight_table`
> + - ``pred_weight_table``
> + -
> + * - __u32
> + - ``dec_ref_pic_marking_bit_size``
> + -
> + * - __u32
> + - ``pic_order_cnt_bit_size``
> + -
> + * - __u8
> + - ``cabac_init_idc``
> + -
> + * - __s8
> + - ``slice_qp_delta``
> + -
> + * - __s8
> + - ``slice_qs_delta``
> + -
> + * - __u8
> + - ``disable_deblocking_filter_idc``
> + -
> + * - __s8
> + - ``slice_alpha_c0_offset_div2``
> + -
> + * - __s8
> + - ``slice_beta_offset_div2``
> + -
> + * - __u8
> + - ``num_ref_idx_l0_active_minus1``
> + -
> + * - __u8
> + - ``num_ref_idx_l1_active_minus1``
> + -
> + * - __u32
> + - ``slice_group_change_cycle``
> + -
> + * - __u8
> + - ``ref_pic_list0[32]``
> + - Reference picture list after applying the per-slic modifications
> + * - __u8
> + - ``ref_pic_list1[32]``
> + - Reference picture list after applying the per-slic modifications
typo: per-slice
> + * - __u32
> + - ``flags``
> + - See :ref:`Slice Parameter Flags <h264_slice_flags>`
> +
> +.. _h264_slice_flags:
> +
> +``Slice Parameter Set Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_H264_SLICE_FLAG_FIELD_PIC``
> + - 0x00000001
> + -
> + * - ``V4L2_H264_SLICE_FLAG_BOTTOM_FIELD``
> + - 0x00000002
> + -
> + * - ``V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED``
> + - 0x00000004
> + -
> + * - ``V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH``
> + - 0x00000008
> + -
> +
> +``Prediction Weight Table``
> +
> + Unless there's a specific comment, refer to the specification for
> + the documentation of these fields, section 7.4.3.2 "Prediction
> + Weight Table Semantics".
Ditto.
> +
> +.. c:type:: v4l2_h264_pred_weight_table
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_h264_pred_weight_table
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u16
> + - ``luma_log2_weight_denom``
> + -
> + * - __u16
> + - ``chroma_log2_weight_denom``
> + -
> + * - struct :c:type:`v4l2_h264_weight_factors`
> + - ``weight_factors[2]``
> + - The weight factors at index 0 are the weight factors for the reference
> + list 0, the one at index 1 for the reference list 1.
> +
> +.. c:type:: v4l2_h264_weight_factors
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_h264_weight_factors
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __s16
> + - ``luma_weight[32]``
> + -
> + * - __s16
> + - ``luma_offset[32]``
> + -
> + * - __s16
> + - ``chroma_weight[32][2]``
> + -
> + * - __s16
> + - ``chroma_offset[32][2]``
> + -
> +
> +``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> + Specifies the decode parameters (as extracted from the bitstream)
> + for the associated H264 slice data. This includes the necessary
> + parameters for configuring a stateless hardware decoding pipeline
> + for H264. The bitstream parameters are defined according to
> + :ref:`h264`. Unless there's a specific comment, refer to the
> + specification for the documentation of these fields.
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_ctrl_h264_decode_param
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_h264_decode_param
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u32
> + - ``num_slices``
> + - Number of slices needed to decode the current frame
> + * - __u16
> + - ``idr_pic_flag``
> + - Is the picture an IDR picture?
Sounds like this could be made a flag to be consistent with how this
kind of fields are represented in the other structs.
Best regards,
Tomasz
Powered by blists - more mailing lists