| 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
| ||
|
Message-ID: <20200806155215.GC1621078@aptenodytes>
Date: Thu, 6 Aug 2020 17:52:15 +0200
From: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To: Ezequiel Garcia <ezequiel@...labora.com>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
Tomasz Figa <tfiga@...omium.org>, kernel@...labora.com,
Jonas Karlman <jonas@...boo.se>,
Hans Verkuil <hverkuil@...all.nl>,
Alexandre Courbot <acourbot@...omium.org>,
Jeffrey Kardatzke <jkardatzke@...omium.org>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Maxime Ripard <mripard@...nel.org>,
Jernej Skrabec <jernej.skrabec@...l.net>
Subject: Re: [PATCH v2 09/14] media: uapi: h264: Clarify SLICE_BASED mode
Hi,
On Thu 06 Aug 20, 12:13, Ezequiel Garcia wrote:
> Currently, the SLICE_BASED and FRAME_BASED modes documentation
> is misleading and not matching the intended use-cases.
>
> Drop non-required fields SLICE_PARAMS 'start_byte_offset' and
> DECODE_PARAMS 'num_slices' and clarify the decoding modes in the
> documentation.
>
> On SLICE_BASED mode, a single slice is expected per OUTPUT buffer,
> and therefore 'start_byte_offset' is not needed (since the offset
> to the slice is the start of the buffer).
Same comment as for the size: how do we plan on managing multiple slices
in a single output buffer later on after removing this?
Cheers,
Paul
> This mode requires the use of CAPTURE buffer holding, and so
> the number of slices shall not be required.
>
> On FRAME_BASED mode, the devices are expected to take care of slice
> parsing. Neither SLICE_PARAMS are required (and shouldn't be
> exposed by frame-based drivers), nor the number of slices.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
> ---
> .../media/v4l/ext-ctrls-codec.rst | 39 +++++--------------
> include/media/h264-ctrls.h | 4 --
> 2 files changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index fff74b7bf32a..b3963c1acc2c 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1748,9 +1748,6 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> 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_params
>
> .. cssclass:: longtable
> @@ -1760,17 +1757,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> :stub-columns: 0
> :widths: 1 1 2
>
> - * - __u32
> - - ``start_byte_offset``
> - Offset (in bytes) from the beginning of the OUTPUT buffer to the start
> - of the slice. If the slice starts with a start code, then this is the
> - offset to such start code. When operating in slice-based decoding mode
> - (see :c:type:`v4l2_mpeg_video_h264_decode_mode`), this field should
> - be set to 0. When operating in frame-based decoding mode, this field
> - should be 0 for the first slice.
> * - __u32
> - ``header_bit_size``
> - -
> + - Offset in bits to slice_data() from the beginning of this slice.
> * - __u32
> - ``first_mb_in_slice``
> -
> @@ -1998,12 +1987,6 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - struct :c:type:`v4l2_h264_dpb_entry`
> - ``dpb[16]``
> -
> - * - __u16
> - - ``num_slices``
> - - Number of slices needed to decode the current frame/field. When
> - operating in slice-based decoding mode (see
> - :c:type:`v4l2_mpeg_video_h264_decode_mode`), this field
> - should always be set to one.
> * - __u16
> - ``nal_ref_idc``
> - NAL reference ID value coming from the NAL Unit header
> @@ -2121,22 +2104,20 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - ``V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED``
> - 0
> - Decoding is done at the slice granularity.
> - In this mode, ``num_slices`` field in struct
> - :c:type:`v4l2_ctrl_h264_decode_params` should be set to 1,
> - and ``start_byte_offset`` in struct
> - :c:type:`v4l2_ctrl_h264_slice_params` should be set to 0.
> The OUTPUT buffer must contain a single slice.
> + When this mode is selected, the ``V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS``
> + control shall be set. When multiple slices compose a frame,
> + use of ``V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF`` flag
> + is required.
> * - ``V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED``
> - 1
> - - Decoding is done at the frame granularity.
> - In this mode, ``num_slices`` field in struct
> - :c:type:`v4l2_ctrl_h264_decode_params` should be set to the number
> - of slices in the frame, and ``start_byte_offset`` in struct
> - :c:type:`v4l2_ctrl_h264_slice_params` should be set accordingly
> - for each slice. For the first slice, ``start_byte_offset`` should
> - be zero.
> + - Decoding is done at the frame granularity,
> The OUTPUT buffer must contain all slices needed to decode the
> frame. The OUTPUT buffer must also contain both fields.
> + This mode will be supported by devices that
> + parse the slice(s) header(s) in hardware. When this mode is
> + selected, the ``V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS``
> + control shall not be set.
>
> ``V4L2_CID_MPEG_VIDEO_H264_START_CODE (enum)``
> Specifies the H264 slice start code expected for each slice.
> diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> index f74736fcfa00..ea8c8c93305b 100644
> --- a/include/media/h264-ctrls.h
> +++ b/include/media/h264-ctrls.h
> @@ -158,9 +158,6 @@ struct v4l2_h264_reference {
> };
>
> struct v4l2_ctrl_h264_slice_params {
> - /* Offset in bytes to the start of slice in the OUTPUT buffer. */
> - __u32 start_byte_offset;
> -
> /* Offset in bits to slice_data() from the beginning of this slice. */
> __u32 header_bit_size;
>
> @@ -219,7 +216,6 @@ struct v4l2_h264_dpb_entry {
>
> struct v4l2_ctrl_h264_decode_params {
> struct v4l2_h264_dpb_entry dpb[V4L2_H264_NUM_DPB_ENTRIES];
> - __u16 num_slices;
> __u16 nal_ref_idc;
> __s32 top_field_order_cnt;
> __s32 bottom_field_order_cnt;
> --
> 2.27.0
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists