From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: mchehab@kernel.org, hverkuil@xs4all.nl,
ezequiel@vanguardiasur.com.ar, p.zabel@pengutronix.de,
gregkh@linuxfoundation.org, mripard@kernel.org,
paul.kocialkowski@bootlin.com, wens@csie.org,
samuel@sholland.org, nicolas.dufresne@collabora.com,
andrzej.p@collabora.com,
Benjamin Gaignard <benjamin.gaignard@collabora.com>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-staging@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev, kernel@collabora.com,
Benjamin Gaignard <benjamin.gaignard@collabora.com>
Subject: Re: [PATCH v6 11/17] media: uapi: Add V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS control
Date: Sun, 29 May 2022 08:40:19 +0200 [thread overview]
Message-ID: <2102641.irdbgypaU6@kista> (raw)
In-Reply-To: <20220527143134.3360174-12-benjamin.gaignard@collabora.com>
Hi!
This series looks very good and I plan to test it shortly on Cedrus, but I
have one major concern below.
Dne petek, 27. maj 2022 ob 16:31:28 CEST je Benjamin Gaignard napisal(a):
> The number of 'entry point offset' can be very variable.
> Instead of using a large static array define a v4l2 dynamic array
> of U32 (V4L2_CTRL_TYPE_U32).
> The number of entry point offsets is reported by the elems field
> and in struct v4l2_ctrl_hevc_slice_params.num_entry_point_offsets
> field.
Slice control by itself is variable length array, so you would actually need
2D variable array for entry points which is not supported. However, easy
workaround for that is to flatten 2D array to 1D and either have another slice
control field which would tell first entry point index for convenience or let
driver calculate it by adding up all num_entry_point_offsets of previous
slices.
Hans, what do you think?
Note, it seems that H265 decoding on Cedrus still works without entry points,
so this problem can be solved later. I'm not sure what we lose with that but
it was suggested that this could influence speed or error resilience or both.
However, I think we're close to solve it, so I'd like to do that now.
Best regards,
Jernej
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> .../userspace-api/media/v4l/ext-ctrls-codec.rst | 11 +++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 5 +++++
> include/media/hevc-ctrls.h | 5 ++++-
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 0796b1563daa..05228e280f66 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -3010,6 +3010,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
> * - __u32
> - ``data_bit_offset``
> - Offset (in bits) to the video data in the current slice data.
> + * - __u32
> + - ``num_entry_point_offsets``
> + - Specifies the number of entry point offset syntax elements in the
slice header.
> * - __u8
> - ``nal_unit_type``
> - Specifies the coding type of the slice (B, P or I).
> @@ -3150,6 +3153,14 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>
> \normalsize
>
> +``V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS (integer)``
> + Specifies entry point offsets in bytes.
> + This control is a dynamically sized array. The number of entry point
> + offsets is reported by the ``elems`` field.
> + This bitstream parameter is defined according to :ref:`hevc`.
> + They are described in section 7.4.7.1 "General slice segment header
> + semantics" of the specification.
> +
> ``V4L2_CID_STATELESS_HEVC_SCALING_MATRIX (struct)``
> Specifies the HEVC scaling matrix parameters used for the scaling
process
> for transform coefficients.
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-
core/v4l2-ctrls-defs.c
> index d594efbcbb93..e22921e7ea61 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1188,6 +1188,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_STATELESS_HEVC_DECODE_PARAMS: return
"HEVC Decode Parameters";
> case V4L2_CID_STATELESS_HEVC_DECODE_MODE: return
"HEVC Decode Mode";
> case V4L2_CID_STATELESS_HEVC_START_CODE: return
"HEVC Start Code";
> + case V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS: return
"HEVC Entry Point Offsets";
>
> /* Colorimetry controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h!
*/
> @@ -1518,6 +1519,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
v4l2_ctrl_type *type,
> case V4L2_CID_STATELESS_HEVC_DECODE_PARAMS:
> *type = V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS;
> break;
> + case V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS:
> + *type = V4L2_CTRL_TYPE_U32;
> + *flags |= V4L2_CTRL_FLAG_DYNAMIC_ARRAY;
> + break;
> case V4L2_CID_STATELESS_VP9_COMPRESSED_HDR:
> *type = V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR;
> break;
> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> index a3c829ef531a..1319cb99ae3f 100644
> --- a/include/media/hevc-ctrls.h
> +++ b/include/media/hevc-ctrls.h
> @@ -20,6 +20,7 @@
> #define V4L2_CID_STATELESS_HEVC_DECODE_PARAMS (V4L2_CID_CODEC_BASE
+ 1012)
> #define V4L2_CID_STATELESS_HEVC_DECODE_MODE (V4L2_CID_CODEC_BASE + 1015)
> #define V4L2_CID_STATELESS_HEVC_START_CODE (V4L2_CID_CODEC_BASE + 1016)
> +#define V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS (V4L2_CID_CODEC_BASE +
1017)
>
> /* enum v4l2_ctrl_type type values */
> #define V4L2_CTRL_TYPE_HEVC_SPS 0x0120
> @@ -318,6 +319,8 @@ struct v4l2_hevc_pred_weight_table {
> *
> * @bit_size: size (in bits) of the current slice data
> * @data_bit_offset: offset (in bits) to the video data in the current slice
data
> + * @num_entry_point_offsets: specifies the number of entry point offset syntax
> + * elements in the slice header.
> * @nal_unit_type: specifies the coding type of the slice (B, P or I)
> * @nuh_temporal_id_plus1: minus 1 specifies a temporal identifier for the
NAL unit
> * @slice_type: see V4L2_HEVC_SLICE_TYPE_{}
> @@ -360,7 +363,7 @@ struct v4l2_hevc_pred_weight_table {
> struct v4l2_ctrl_hevc_slice_params {
> __u32 bit_size;
> __u32 data_bit_offset;
> -
> + __u32 num_entry_point_offsets;
> /* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> __u8 nal_unit_type;
> __u8 nuh_temporal_id_plus1;
> --
> 2.32.0
>
>
next prev parent reply other threads:[~2022-05-29 6:40 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-27 14:31 [PATCH v6 00/17] Move HEVC stateless controls out of staging Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 01/17] videodev2.h: add V4L2_CTRL_FLAG_DYNAMIC_ARRAY Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 02/17] v4l2-ctrls: add support for dynamically allocated arrays Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 03/17] vivid: add dynamic array test control Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 04/17] media: uapi: HEVC: Add missing fields in HEVC controls Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 05/17] media: uapi: HEVC: Rename HEVC stateless controls with STATELESS prefix Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 06/17] media: uapi: HEVC: Change pic_order_cnt definition in v4l2_hevc_dpb_entry Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 07/17] media: uapi: HEVC: Add SEI pic struct flags Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 08/17] media: uapi: HEVC: Add documentation to uAPI structure Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 09/17] media: uapi: HEVC: Define V4L2_CID_STATELESS_HEVC_SLICE_PARAMS as a dynamic array Benjamin Gaignard
2022-05-29 9:19 ` Jernej Škrabec
2022-06-01 15:43 ` Jernej Škrabec
2022-06-01 16:02 ` Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 10/17] media: uapi: Move parsed HEVC pixel format out of staging Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 11/17] media: uapi: Add V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS control Benjamin Gaignard
2022-05-29 6:40 ` Jernej Škrabec [this message]
2022-05-30 9:18 ` Hans Verkuil
2022-05-30 13:49 ` Hans Verkuil
2022-05-30 21:24 ` Jernej Škrabec
2022-05-31 6:58 ` Benjamin Gaignard
2022-05-31 18:20 ` Jernej Škrabec
2022-06-01 16:20 ` Nicolas Dufresne
2022-06-01 16:35 ` Jernej Škrabec
2022-06-01 17:07 ` Nicolas Dufresne
2022-06-10 14:08 ` John Cox
2022-05-27 14:31 ` [PATCH v6 12/17] media: uapi: Move the HEVC stateless control type out of staging Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 13/17] media: controls: Log HEVC stateless control in .std_log Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 14/17] media: hantro: Stop using Hantro dedicated control Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 15/17] media: uapi: HEVC: fix padding in v4l2 control structures Benjamin Gaignard
2022-05-27 14:31 ` [PATCH v6 16/17] media: uapi: Change data_bit_offset definition Benjamin Gaignard
2022-05-29 6:45 ` Jernej Škrabec
2022-06-01 16:17 ` Jernej Škrabec
2022-06-01 16:33 ` Benjamin Gaignard
2022-06-12 20:40 ` Jernej Škrabec
2022-06-13 18:17 ` Jernej Škrabec
2022-05-27 14:31 ` [PATCH v6 17/17] media: uapi: move HEVC stateless controls out of staging Benjamin Gaignard
2022-06-10 14:00 ` [PATCH v6 00/17] Move " John Cox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2102641.irdbgypaU6@kista \
--to=jernej.skrabec@gmail.com \
--cc=andrzej.p@collabora.com \
--cc=benjamin.gaignard@collabora.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil@xs4all.nl \
--cc=kernel@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-sunxi@lists.linux.dev \
--cc=mchehab@kernel.org \
--cc=mripard@kernel.org \
--cc=nicolas.dufresne@collabora.com \
--cc=p.zabel@pengutronix.de \
--cc=paul.kocialkowski@bootlin.com \
--cc=samuel@sholland.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).