| 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: <cgihahl6gu9089b1ph9tj4vobftaoatbjt@4ax.com>
Date: Tue, 14 Jun 2022 18:50:36 +0100
From: John Cox <jc@...esim.co.uk>
To: Jernej Škrabec <jernej.skrabec@...il.com>
Cc: Benjamin Gaignard <benjamin.gaignard@...labora.com>,
mchehab@...nel.org, ezequiel@...guardiasur.com.ar,
p.zabel@...gutronix.de, gregkh@...uxfoundation.org,
mripard@...nel.org, paul.kocialkowski@...tlin.com, wens@...e.org,
samuel@...lland.org, nicolas.dufresne@...labora.com,
andrzej.p@...labora.com, Hans Verkuil <hverkuil@...all.nl>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
kernel@...labora.com
Subject: Re: [PATCH v8 09/17] media: uapi: HEVC: Define V4L2_CID_STATELESS_HEVC_SLICE_PARAMS as a dynamic array
Hi
>Dne torek, 14. junij 2022 ob 15:50:23 CEST je Hans Verkuil napisal(a):
>> On 6/14/22 10:36, Benjamin Gaignard wrote:
>> > Make explicit that V4L2_CID_STATELESS_HEVC_SLICE_PARAMS control is
>> > a dynamic array control type.
>> > Some drivers may be able to receive multiple slices in one control
>> > to improve decoding performance.
>> >
>> > Define the max size of the dynamic that can driver can set in .dims = {}.
>> >
>> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
>> > ---
>> > version 7:
>> > - Add Jernej patch to set array dims for Cedrus
>> >
>> > version 6:
>> > - Set V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag automatically when using
>> > V4L2_CID_STATELESS_HEVC_SLICE_PARAMS control.
>> > - Add a define for max slices count
>> >
>> > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 2 ++
>> > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
>> > drivers/staging/media/sunxi/cedrus/cedrus.c | 1 +
>> > include/media/hevc-ctrls.h | 5 +++++
>> > 4 files changed, 9 insertions(+)
>> >
>> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
>Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> > index 06b967de140c..0796b1563daa 100644
>> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> > @@ -2986,6 +2986,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>> > These bitstream parameters are defined according to :ref:`hevc`.
>> > They are described in section 7.4.7 "General slice segment header
>> > semantics" of the specification.
>> > + This control is a dynamically sized 1-dimensional array,
>> > + V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag must be set when using it.
>> >
>> > .. c:type:: v4l2_ctrl_hevc_slice_params
>> >
>> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/
>v4l2-core/v4l2-ctrls-defs.c
>> > index 9f55503cd3d6..d594efbcbb93 100644
>> > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>> > @@ -1510,6 +1510,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
>v4l2_ctrl_type *type,
>> > break;
>> > case V4L2_CID_STATELESS_HEVC_SLICE_PARAMS:
>> > *type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
>> > + *flags |= V4L2_CTRL_FLAG_DYNAMIC_ARRAY;
>> > break;
>> > case V4L2_CID_STATELESS_HEVC_SCALING_MATRIX:
>> > *type = V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX;
>> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/
>media/sunxi/cedrus/cedrus.c
>> > index 87be975a72b6..f3391c7c811c 100644
>> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
>> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
>> > @@ -178,6 +178,7 @@ static const struct cedrus_control cedrus_controls[] =
>{
>> > {
>> > .cfg = {
>> > .id =
>V4L2_CID_STATELESS_HEVC_SLICE_PARAMS,
>> > + .dims = { 1 },
>>
>> So cedrus HW supports only a single SLICE_PARAMS struct? Perhaps add that
>> as a comment.
>
>Yes, for now. I have WIP patch which will remove this limitation.
>
>Please corrrect me if I'm wrong, but this dimension represents maximum allowed
>array length? If so, this is a bit awkward for cases where there is really no
>limit. Like in this case. Cedrus can process only one slice which means that
>driver will have to loop over all submitted slices, one by one. Thus, there is
>no real limitation. It could be set to 10, 1000 or even more. Any suggestion
>for appropriate upper limit is appreciated.
Likewise for Pi there is no real upper limit.
Regards
John Cox
>Best regards,
>Jernej
>
>>
>> > },
>> > .codec = CEDRUS_CODEC_H265,
>> > },
>> > diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>> > index 0dbd5d681c28..140151609c96 100644
>> > --- a/include/media/hevc-ctrls.h
>> > +++ b/include/media/hevc-ctrls.h
>> > @@ -311,9 +311,14 @@ struct v4l2_hevc_pred_weight_table {
>> > #define
>V4L2_HEVC_SLICE_PARAMS_FLAG_SLICE_LOOP_FILTER_ACROSS_SLICES_ENABLED (1ULL <<
>8)
>> > #define V4L2_HEVC_SLICE_PARAMS_FLAG_DEPENDENT_SLICE_SEGMENT (1ULL <<
>9)
>> >
>> > +#define V4L2_HEVC_SLICE_MAX_COUNT 600
>> > +
>>
>> I would drop this define. It is not used at the moment, and I think it
>> is the driver that should set this through the dims field (as cedrus
>> does above). It is likely to be limited by hardware anyway (just my guess).
>>
>> If it is needed in the future, it can always be added.
>>
>> Regards,
>>
>> Hans
>>
>> > /**
>> > * v4l2_ctrl_hevc_slice_params - HEVC slice parameters
>> > *
>> > + * This control is a dynamically sized 1-dimensional array,
>> > + * V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag must be set when using it.
>> > + *
>> > * @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
>> > * @nal_unit_type: specifies the coding type of the slice (B, P or I)
>>
>>
>
Powered by blists - more mailing lists