| Message ID | 20220527143134.3360174-1-benjamin.gaignard@collabora.com |
|---|---|
| Headers | show |
| Series | Move HEVC stateless controls out of staging | expand |
Dne ponedeljek, 30. maj 2022 ob 15:49:57 CEST je Hans Verkuil napisal(a): > On 30/05/2022 11:18, Hans Verkuil wrote: > > On 29/05/2022 08:40, Jernej Škrabec wrote: > >> 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? > > > > If I would support 2D variable array sizes, wouldn't that be more elegant? > > > > The current implementation doesn't support that, but as the commit log for > > patch 1/17 says: > > > > "Currently dynamically sized arrays are limited to one dimensional arrays, > > but that might change in the future if there is a need for it." > > > > Let me know if you agree, and I'll try to implement this. It's been a while > > since I last looked at this, so I'm not sure how much work it is, but it is > > probably worth a shot. > > Digging more into this made me realize that this doesn't actually help for this > particular case. > > I would lean towards your second suggestion of adding up all num_entry_point_offsets > of previous slices. Just one question/clarification about dynamic arrays - does driver need to reserve maximum amount of memory for dynamic array control at initialization time? If so, this would still be problematic, since there cound be a huge amount of entry points in theory. Best regards, Jernej > > Regards, > > Hans > > > > > Regards, > > > > Hans > > > >> > >> 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 > >>> > >>> > >> > >> > > > >
Le 30/05/2022 à 23:24, Jernej Škrabec a écrit : > Dne ponedeljek, 30. maj 2022 ob 15:49:57 CEST je Hans Verkuil napisal(a): >> On 30/05/2022 11:18, Hans Verkuil wrote: >>> On 29/05/2022 08:40, Jernej Škrabec wrote: >>>> 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? >>> If I would support 2D variable array sizes, wouldn't that be more elegant? >>> >>> The current implementation doesn't support that, but as the commit log for >>> patch 1/17 says: >>> >>> "Currently dynamically sized arrays are limited to one dimensional arrays, >>> but that might change in the future if there is a need for it." >>> >>> Let me know if you agree, and I'll try to implement this. It's been a > while >>> since I last looked at this, so I'm not sure how much work it is, but it > is >>> probably worth a shot. >> Digging more into this made me realize that this doesn't actually help for > this >> particular case. >> >> I would lean towards your second suggestion of adding up all > num_entry_point_offsets >> of previous slices. > Just one question/clarification about dynamic arrays - does driver need to > reserve maximum amount of memory for dynamic array control at initialization > time? If so, this would still be problematic, since there cound be a huge > amount of entry points in theory. When adding the control the driver could set .dims field to specify the max number of accepted slices. I have added '#define V4L2_HEVC_SLICE_MAX_COUNT 600' that you could use for this field. It is the value we have found when using slices with RKVDEC driver. Regards, Benjamin > > Best regards, > Jernej > >> Regards, >> >> Hans >> >>> Regards, >>> >>> Hans >>> >>>> 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 >>>>> >>>>> >>>> >> >
Dne torek, 31. maj 2022 ob 08:58:46 CEST je Benjamin Gaignard napisal(a): > > Le 30/05/2022 à 23:24, Jernej Škrabec a écrit : > > Dne ponedeljek, 30. maj 2022 ob 15:49:57 CEST je Hans Verkuil napisal(a): > >> On 30/05/2022 11:18, Hans Verkuil wrote: > >>> On 29/05/2022 08:40, Jernej Škrabec wrote: > >>>> 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? > >>> If I would support 2D variable array sizes, wouldn't that be more elegant? > >>> > >>> The current implementation doesn't support that, but as the commit log for > >>> patch 1/17 says: > >>> > >>> "Currently dynamically sized arrays are limited to one dimensional arrays, > >>> but that might change in the future if there is a need for it." > >>> > >>> Let me know if you agree, and I'll try to implement this. It's been a > > while > >>> since I last looked at this, so I'm not sure how much work it is, but it > > is > >>> probably worth a shot. > >> Digging more into this made me realize that this doesn't actually help for > > this > >> particular case. > >> > >> I would lean towards your second suggestion of adding up all > > num_entry_point_offsets > >> of previous slices. > > Just one question/clarification about dynamic arrays - does driver need to > > reserve maximum amount of memory for dynamic array control at initialization > > time? If so, this would still be problematic, since there cound be a huge > > amount of entry points in theory. > > When adding the control the driver could set .dims field to specify > the max number of accepted slices. > I have added '#define V4L2_HEVC_SLICE_MAX_COUNT 600' that you could use > for this field. It is the value we have found when using slices with RKVDEC > driver. Is this maximum value applicable only to RKVDEC or is universal? Anyway, this means maximum offset points control for Cedrus would be 600 * 1024 (max. offset points supported per slice) * 4 ~= 2.4 MB, which is a lot for one control, but I can live with that... Best regards, Jernej > > Regards, > Benjamin > > > > > Best regards, > > Jernej > > > >> Regards, > >> > >> Hans > >> > >>> Regards, > >>> > >>> Hans > >>> > >>>> 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 > >>>>> > >>>>> > >>>> > >> > > >
Le mardi 31 mai 2022 à 20:20 +0200, Jernej Škrabec a écrit : > Dne torek, 31. maj 2022 ob 08:58:46 CEST je Benjamin Gaignard napisal(a): > > > > Le 30/05/2022 à 23:24, Jernej Škrabec a écrit : > > > Dne ponedeljek, 30. maj 2022 ob 15:49:57 CEST je Hans Verkuil napisal(a): > > > > On 30/05/2022 11:18, Hans Verkuil wrote: > > > > > On 29/05/2022 08:40, Jernej Škrabec wrote: > > > > > > 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? > > > > > If I would support 2D variable array sizes, wouldn't that be more > elegant? > > > > > > > > > > The current implementation doesn't support that, but as the commit log > for > > > > > patch 1/17 says: > > > > > > > > > > "Currently dynamically sized arrays are limited to one dimensional > arrays, > > > > > but that might change in the future if there is a need for it." > > > > > > > > > > Let me know if you agree, and I'll try to implement this. It's been a > > > while > > > > > since I last looked at this, so I'm not sure how much work it is, but it > > > is > > > > > probably worth a shot. > > > > Digging more into this made me realize that this doesn't actually help > for > > > this > > > > particular case. > > > > > > > > I would lean towards your second suggestion of adding up all > > > num_entry_point_offsets > > > > of previous slices. > > > Just one question/clarification about dynamic arrays - does driver need to > > > reserve maximum amount of memory for dynamic array control at > initialization > > > time? If so, this would still be problematic, since there cound be a huge > > > amount of entry points in theory. > > > > When adding the control the driver could set .dims field to specify > > the max number of accepted slices. > > I have added '#define V4L2_HEVC_SLICE_MAX_COUNT 600' that you could use > > for this field. It is the value we have found when using slices with RKVDEC > > driver. > > Is this maximum value applicable only to RKVDEC or is universal? Anyway, this > means maximum offset points control for Cedrus would be 600 * 1024 (max. offset > points supported per slice) * 4 ~= 2.4 MB, which is a lot for one control, but > I can live with that... I believe its defined following "Table A.8 – General tier and level limits". With the assumption there will never be a level 7 (which I think is fair). If anyone saw other reasons for this limit, let me know. This is a worse case scenario, this is quite unlikely in practice, so while performance might be a disaster if your craft a stream for that case, I don't think it will ever happen in real life. > > Best regards, > Jernej > > > > > Regards, > > Benjamin > > > > > > > > Best regards, > > > Jernej > > > > > > > Regards, > > > > > > > > Hans > > > > > > > > > Regards, > > > > > > > > > > Hans > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Dne sreda, 01. junij 2022 ob 18:20:53 CEST je Nicolas Dufresne napisal(a): > Le mardi 31 mai 2022 à 20:20 +0200, Jernej Škrabec a écrit : > > Dne torek, 31. maj 2022 ob 08:58:46 CEST je Benjamin Gaignard napisal(a): > > > Le 30/05/2022 à 23:24, Jernej Škrabec a écrit : > > > > Dne ponedeljek, 30. maj 2022 ob 15:49:57 CEST je Hans Verkuil napisal(a): > > > > > On 30/05/2022 11:18, Hans Verkuil wrote: > > > > > > On 29/05/2022 08:40, Jernej Škrabec wrote: > > > > > > > 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? > > > > > > > > > > > > If I would support 2D variable array sizes, wouldn't that be more > > > > elegant? > > > > > > > > The current implementation doesn't support that, but as the commit > > > > > > log > > > > for > > > > > > > > patch 1/17 says: > > > > > > > > > > > > "Currently dynamically sized arrays are limited to one dimensional > > > > arrays, > > > > > > > > but that might change in the future if there is a need for it." > > > > > > > > > > > > Let me know if you agree, and I'll try to implement this. It's > > > > > > been a > > > > > > > > while > > > > > > > > > > since I last looked at this, so I'm not sure how much work it is, > > > > > > but it > > > > > > > > is > > > > > > > > > > probably worth a shot. > > > > > > > > > > Digging more into this made me realize that this doesn't actually > > > > > help > > > > for > > > > > > this > > > > > > > > > particular case. > > > > > > > > > > I would lean towards your second suggestion of adding up all > > > > > > > > num_entry_point_offsets > > > > > > > > > of previous slices. > > > > > > > > Just one question/clarification about dynamic arrays - does driver > > > > need to > > > > reserve maximum amount of memory for dynamic array control at > > > > initialization > > > > > > time? If so, this would still be problematic, since there cound be a > > > > huge > > > > amount of entry points in theory. > > > > > > When adding the control the driver could set .dims field to specify > > > the max number of accepted slices. > > > I have added '#define V4L2_HEVC_SLICE_MAX_COUNT 600' that you could use > > > for this field. It is the value we have found when using slices with > > > RKVDEC > > > driver. > > > > Is this maximum value applicable only to RKVDEC or is universal? Anyway, > > this means maximum offset points control for Cedrus would be 600 * 1024 > > (max. offset points supported per slice) * 4 ~= 2.4 MB, which is a lot > > for one control, but I can live with that... > > I believe its defined following "Table A.8 – General tier and level limits". > With the assumption there will never be a level 7 (which I think is fair). > If anyone saw other reasons for this limit, let me know. > > This is a worse case scenario, this is quite unlikely in practice, so while > performance might be a disaster if your craft a stream for that case, I > don't think it will ever happen in real life. But do we really need to cover worst case scenario? In theory, one driver can set limit to (for example) max 100 slices and if there is a frame with 600 slices, userspace app would submit 6 decode requests. Basically the same way it's done today. While not as performant, it would be good compromise between resources and speed. > > > Best regards, > > Jernej > > > > > Regards, > > > Benjamin > > > > > > > Best regards, > > > > Jernej > > > > > > > > > Regards, > > > > > > > > > > Hans > > > > > > > > > > > Regards, > > > > > > > > > > > > Hans > > > > > > > > > > > > > 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;
This series aims to make HEVC uapi stable and usable for hardware decoder. HEVC uapi is used by 2 mainlined drivers (Cedrus and Hantro) and 2 out of the tree drivers (rkvdec and RPI). version 6: - Add short_term_ref_pic_set_size and long_term_ref_pic_set_size in v4l2_ctrl_hevc_decode_params structure. - Change slice_pic_order_cnt type to s32 to match with PoC type. - Set V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag automatically when using V4L2_CID_STATELESS_HEVC_SLICE_PARAMS control. - Add a define for max slices count - Stop using Hantro dedicated control. This version has been tested with these branches: - GStreamer: https://gitlab.freedesktop.org/benjamin.gaignard1/gstreamer/-/tree/HEVC_aligned_with_kernel_5.15 - Linux: https://gitlab.collabora.com/benjamin.gaignard/for-upstream/-/tree/HEVC_UAPI_V6 With patches to decode 10-bits bitstream and produce P010 frames the Fluster score which was 77/147 before, is now 138/147. The 10-bits series will comes after this because of it dependency to uAPI change. If you are curious you can find the WIP branch here: https://gitlab.collabora.com/benjamin.gaignard/for-upstream/-/commits/WIP_HEVC_UAPI_V6 The 9 failing tests are: - CONFWIN_A_Sony_1 which contains conformance_window_flag that isn't supported by the hardware (but visually ok aside a pixel shift). - PICSIZE_{A,B,C,D}_Bossen_1 where resolutions are to big for Hantro hardware. - TSKIP_A_MS_3 is ok when testing alone but fail (corrupted lines on the first frame) when running it after a couple of other tests. - VPSSPSPPS_A_MainConcept_1 where there is an issue on gst parser side because of VPS/SPS/PPS ordering - WPP_D_ericsson_MAIN_2 and WPP_D_ericsson_MAIN10_2 are visually ok but some difference exist on 5 decoded frames. Some pixels values are no the same the very end of few lines. version 6: - Stop using Hantro dedicated control and compute the number of bytes to skip inside the driver. - Rebased on media_tree/master version 5: - Change __u16 pic_order_cnt[2] into __s32 pic_order_cnt_val in hevc_dpb_entry structure - Add defines for SEI pic_struct values (patch 4) - Fix numbers of bits computation in cedrus_h265_skip_bits() parameters - Fix num_short_term_ref_pic_sets and num_long_term_ref_pics_sps documentation (patch 8) - Rebased on v5-18-rc1 GStreamer H265 decoder plugin aligned with HEVC uAPI v5: https://gitlab.freedesktop.org/benjamin.gaignard1/gstreamer/-/tree/HEVC_aligned_with_kernel_5.15 Version 4: - Add num_entry_point_offsets field in struct v4l2_ctrl_hevc_slice_params - Fix V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS name - Initialize control V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS - Fix space/tab issue in kernel-doc - Add patch to change data_bit_offset definition - Fix hantro-media SPDX license - put controls under stateless section in v4l2-ctrls-defs.c At the end fluster tests results on IMX8MQ is 77/147 for HEVC codec. Benjamin Gaignard (14): media: uapi: HEVC: Add missing fields in HEVC controls media: uapi: HEVC: Rename HEVC stateless controls with STATELESS prefix media: uapi: HEVC: Change pic_order_cnt definition in v4l2_hevc_dpb_entry media: uapi: HEVC: Add SEI pic struct flags media: uapi: HEVC: Add documentation to uAPI structure media: uapi: HEVC: Define V4L2_CID_STATELESS_HEVC_SLICE_PARAMS as a dynamic array media: uapi: Move parsed HEVC pixel format out of staging media: uapi: Add V4L2_CID_STATELESS_HEVC_ENTRY_POINT_OFFSETS control media: uapi: Move the HEVC stateless control type out of staging media: controls: Log HEVC stateless control in .std_log media: hantro: Stop using Hantro dedicated control media: uapi: HEVC: fix padding in v4l2 control structures media: uapi: Change data_bit_offset definition media: uapi: move HEVC stateless controls out of staging Hans Verkuil (3): videodev2.h: add V4L2_CTRL_FLAG_DYNAMIC_ARRAY v4l2-ctrls: add support for dynamically allocated arrays. vivid: add dynamic array test control .../media/v4l/ext-ctrls-codec-stateless.rst | 897 ++++++++++++++++++ .../media/v4l/ext-ctrls-codec.rst | 780 --------------- .../media/v4l/pixfmt-compressed.rst | 7 +- .../media/v4l/vidioc-g-ext-ctrls.rst | 20 + .../media/v4l/vidioc-queryctrl.rst | 8 + .../media/videodev2.h.rst.exceptions | 5 + .../media/test-drivers/vivid/vivid-ctrls.c | 15 + drivers/media/v4l2-core/v4l2-ctrls-api.c | 103 +- drivers/media/v4l2-core/v4l2-ctrls-core.c | 206 +++- drivers/media/v4l2-core/v4l2-ctrls-defs.c | 38 +- drivers/media/v4l2-core/v4l2-ctrls-priv.h | 3 +- drivers/media/v4l2-core/v4l2-ctrls-request.c | 13 +- drivers/staging/media/hantro/hantro_drv.c | 62 +- .../staging/media/hantro/hantro_g2_hevc_dec.c | 69 +- drivers/staging/media/hantro/hantro_hevc.c | 10 +- drivers/staging/media/hantro/hantro_hw.h | 4 +- drivers/staging/media/sunxi/cedrus/cedrus.c | 24 +- .../staging/media/sunxi/cedrus/cedrus_dec.c | 10 +- .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- include/media/hevc-ctrls.h | 250 ----- include/media/v4l2-ctrls.h | 48 +- include/uapi/linux/v4l2-controls.h | 458 +++++++++ include/uapi/linux/videodev2.h | 13 + 23 files changed, 1830 insertions(+), 1219 deletions(-) delete mode 100644 include/media/hevc-ctrls.h