diff mbox series

[v6,16/17] media: uapi: Change data_bit_offset definition

Message ID 20220527143134.3360174-17-benjamin.gaignard@collabora.com (mailing list archive)
State New, archived
Headers show
Series Move HEVC stateless controls out of staging | expand

Commit Message

Benjamin Gaignard May 27, 2022, 2:31 p.m. UTC
'F.7.3.6.1 General slice segment header syntax' section of HEVC
specification describes that a slice header always end aligned on
byte boundary, therefore we only need to provide the data offset in bytes.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
 drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
 include/media/hevc-ctrls.h                                | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Jernej Škrabec May 29, 2022, 6:45 a.m. UTC | #1
Dne petek, 27. maj 2022 ob 16:31:33 CEST je Benjamin Gaignard napisal(a):
> 'F.7.3.6.1 General slice segment header syntax' section of HEVC
> specification describes that a slice header always end aligned on
> byte boundary, therefore we only need to provide the data offset in bytes.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
>  drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
>  include/media/hevc-ctrls.h                                | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 48a8825a001b..37079581c661 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -3008,8 +3008,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>        - ``bit_size``
>        - Size (in bits) of the current slice data.
>      * - __u32
> -      - ``data_bit_offset``
> -      - Offset (in bits) to the video data in the current slice data.
> +      - ``data_byte_offset``
> +      - Offset (in bytes) 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.
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/
staging/media/sunxi/cedrus/cedrus_h265.c
> index 411601975124..835454239f73 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -405,7 +405,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
>  	/* Initialize bitstream access. */
>  	cedrus_write(dev, VE_DEC_H265_TRIGGER, 
VE_DEC_H265_TRIGGER_INIT_SWDEC);
>  
> -	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
> +	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);

While it's true that actual data starts on 8-bit aligned address, Cedrus for 
some reason needs offset which points at the end of the header, before 
alignment. There is very simple way to determine that, but unfortunately this 
means reading source buffer.

In short, above code won't work. I'll provide a fix.

Best regards,
Jernej

>  
>  	/* Bitstream parameters. */
>  
> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> index 9abca1a75bd4..936ff693967b 100644
> --- a/include/media/hevc-ctrls.h
> +++ b/include/media/hevc-ctrls.h
> @@ -312,7 +312,7 @@ struct v4l2_hevc_pred_weight_table {
>   * 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
> + * @data_byte_offset: offset (in bytes) 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)
> @@ -356,7 +356,7 @@ struct v4l2_hevc_pred_weight_table {
>   */
>  struct v4l2_ctrl_hevc_slice_params {
>  	__u32	bit_size;
> -	__u32	data_bit_offset;
> +	__u32	data_byte_offset;
>  	__u32	num_entry_point_offsets;
>  	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
>  	__u8	nal_unit_type;
> -- 
> 2.32.0
> 
>
Jernej Škrabec June 1, 2022, 4:17 p.m. UTC | #2
Dne nedelja, 29. maj 2022 ob 08:45:57 CEST je Jernej Škrabec napisal(a):
> Dne petek, 27. maj 2022 ob 16:31:33 CEST je Benjamin Gaignard napisal(a):
> > 'F.7.3.6.1 General slice segment header syntax' section of HEVC
> > specification describes that a slice header always end aligned on
> > byte boundary, therefore we only need to provide the data offset in bytes.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> >  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
> >  drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
> >  include/media/hevc-ctrls.h                                | 4 ++--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 48a8825a001b..37079581c661 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -3008,8 +3008,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
> >        - ``bit_size``
> >        - Size (in bits) of the current slice data.
> >      * - __u32
> > -      - ``data_bit_offset``
> > -      - Offset (in bits) to the video data in the current slice data.
> > +      - ``data_byte_offset``
> > +      - Offset (in bytes) 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.
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/
> staging/media/sunxi/cedrus/cedrus_h265.c
> > index 411601975124..835454239f73 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > @@ -405,7 +405,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
> >  	/* Initialize bitstream access. */
> >  	cedrus_write(dev, VE_DEC_H265_TRIGGER, 
> VE_DEC_H265_TRIGGER_INIT_SWDEC);
> >  
> > -	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
> > +	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);
> 
> While it's true that actual data starts on 8-bit aligned address, Cedrus for 
> some reason needs offset which points at the end of the header, before 
> alignment. There is very simple way to determine that, but unfortunately 
this 
> means reading source buffer.
> 
> In short, above code won't work. I'll provide a fix.

Please include following fix http://ix.io/3Z8x otherwise Cedrus will fail to 
decode slice.

Other than fix in previous e-mail and this one, code looks good and I'll be 
able to add missing functionality to Cedrus without much trouble in follow up 
series.

Best regards,
Jernej

> 
> >  
> >  	/* Bitstream parameters. */
> >  
> > diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> > index 9abca1a75bd4..936ff693967b 100644
> > --- a/include/media/hevc-ctrls.h
> > +++ b/include/media/hevc-ctrls.h
> > @@ -312,7 +312,7 @@ struct v4l2_hevc_pred_weight_table {
> >   * 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
> > + * @data_byte_offset: offset (in bytes) 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)
> > @@ -356,7 +356,7 @@ struct v4l2_hevc_pred_weight_table {
> >   */
> >  struct v4l2_ctrl_hevc_slice_params {
> >  	__u32	bit_size;
> > -	__u32	data_bit_offset;
> > +	__u32	data_byte_offset;
> >  	__u32	num_entry_point_offsets;
> >  	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> >  	__u8	nal_unit_type;
> > -- 
> > 2.32.0
> > 
> > 
> 
> 
>
Benjamin Gaignard June 1, 2022, 4:33 p.m. UTC | #3
Le 01/06/2022 à 18:17, Jernej Škrabec a écrit :
> Dne nedelja, 29. maj 2022 ob 08:45:57 CEST je Jernej Škrabec napisal(a):
>> Dne petek, 27. maj 2022 ob 16:31:33 CEST je Benjamin Gaignard napisal(a):
>>> 'F.7.3.6.1 General slice segment header syntax' section of HEVC
>>> specification describes that a slice header always end aligned on
>>> byte boundary, therefore we only need to provide the data offset in bytes.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>>> ---
>>>   Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
>>>   drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
>>>   include/media/hevc-ctrls.h                                | 4 ++--
>>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
>> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> index 48a8825a001b..37079581c661 100644
>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> @@ -3008,8 +3008,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>>         - ``bit_size``
>>>         - Size (in bits) of the current slice data.
>>>       * - __u32
>>> -      - ``data_bit_offset``
>>> -      - Offset (in bits) to the video data in the current slice data.
>>> +      - ``data_byte_offset``
>>> +      - Offset (in bytes) 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.
>>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/
>> staging/media/sunxi/cedrus/cedrus_h265.c
>>> index 411601975124..835454239f73 100644
>>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
>>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
>>> @@ -405,7 +405,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
>>>   	/* Initialize bitstream access. */
>>>   	cedrus_write(dev, VE_DEC_H265_TRIGGER,
>> VE_DEC_H265_TRIGGER_INIT_SWDEC);
>>>   
>>> -	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
>>> +	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);
>> While it's true that actual data starts on 8-bit aligned address, Cedrus for
>> some reason needs offset which points at the end of the header, before
>> alignment. There is very simple way to determine that, but unfortunately
> this
>> means reading source buffer.
>>
>> In short, above code won't work. I'll provide a fix.
> Please include following fix http://ix.io/3Z8x otherwise Cedrus will fail to
> decode slice.
>
> Other than fix in previous e-mail and this one, code looks good and I'll be
> able to add missing functionality to Cedrus without much trouble in follow up
> series.

Thanks for the patch it will be in version 7.

Regards,
Benjamin

>
> Best regards,
> Jernej
>
>>>   
>>>   	/* Bitstream parameters. */
>>>   
>>> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>>> index 9abca1a75bd4..936ff693967b 100644
>>> --- a/include/media/hevc-ctrls.h
>>> +++ b/include/media/hevc-ctrls.h
>>> @@ -312,7 +312,7 @@ struct v4l2_hevc_pred_weight_table {
>>>    * 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
>>> + * @data_byte_offset: offset (in bytes) 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)
>>> @@ -356,7 +356,7 @@ struct v4l2_hevc_pred_weight_table {
>>>    */
>>>   struct v4l2_ctrl_hevc_slice_params {
>>>   	__u32	bit_size;
>>> -	__u32	data_bit_offset;
>>> +	__u32	data_byte_offset;
>>>   	__u32	num_entry_point_offsets;
>>>   	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
>>>   	__u8	nal_unit_type;
>>> -- 
>>> 2.32.0
>>>
>>>
>>
>>
>
Jernej Škrabec June 12, 2022, 8:40 p.m. UTC | #4
Dne sreda, 01. junij 2022 ob 18:33:22 CEST je Benjamin Gaignard napisal(a):
> 
> Le 01/06/2022 à 18:17, Jernej Škrabec a écrit :
> > Dne nedelja, 29. maj 2022 ob 08:45:57 CEST je Jernej Škrabec napisal(a):
> >> Dne petek, 27. maj 2022 ob 16:31:33 CEST je Benjamin Gaignard napisal(a):
> >>> 'F.7.3.6.1 General slice segment header syntax' section of HEVC
> >>> specification describes that a slice header always end aligned on
> >>> byte boundary, therefore we only need to provide the data offset in 
bytes.
> >>>
> >>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> >>> ---
> >>>   Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
> >>>   drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
> >>>   include/media/hevc-ctrls.h                                | 4 ++--
> >>>   3 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/
> >> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> >>> index 48a8825a001b..37079581c661 100644
> >>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> >>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> >>> @@ -3008,8 +3008,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
> >>>         - ``bit_size``
> >>>         - Size (in bits) of the current slice data.
> >>>       * - __u32
> >>> -      - ``data_bit_offset``
> >>> -      - Offset (in bits) to the video data in the current slice data.
> >>> +      - ``data_byte_offset``
> >>> +      - Offset (in bytes) 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.
> >>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/
> >> staging/media/sunxi/cedrus/cedrus_h265.c
> >>> index 411601975124..835454239f73 100644
> >>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> >>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> >>> @@ -405,7 +405,7 @@ static void cedrus_h265_setup(struct cedrus_ctx 
*ctx,
> >>>   	/* Initialize bitstream access. */
> >>>   	cedrus_write(dev, VE_DEC_H265_TRIGGER,
> >> VE_DEC_H265_TRIGGER_INIT_SWDEC);
> >>>   
> >>> -	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
> >>> +	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);
> >> While it's true that actual data starts on 8-bit aligned address, Cedrus 
for
> >> some reason needs offset which points at the end of the header, before
> >> alignment. There is very simple way to determine that, but unfortunately
> > this
> >> means reading source buffer.
> >>
> >> In short, above code won't work. I'll provide a fix.
> > Please include following fix http://ix.io/3Z8x otherwise Cedrus will fail 
to
> > decode slice.

Fluster testing show that this patch isn't completely fine. I also have some 
other issues, which need control values comparisons. It would be best, if you 
can wait until I finish comparison. Old, hackish HEVC patches have high fluster 
score whereas new, based on stable uAPI, has low and CPU also locks up...

Best regards,
Jernej

> >
> > Other than fix in previous e-mail and this one, code looks good and I'll be
> > able to add missing functionality to Cedrus without much trouble in follow 
up
> > series.
> 
> Thanks for the patch it will be in version 7.
> 
> Regards,
> Benjamin
> 
> >
> > Best regards,
> > Jernej
> >
> >>>   
> >>>   	/* Bitstream parameters. */
> >>>   
> >>> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> >>> index 9abca1a75bd4..936ff693967b 100644
> >>> --- a/include/media/hevc-ctrls.h
> >>> +++ b/include/media/hevc-ctrls.h
> >>> @@ -312,7 +312,7 @@ struct v4l2_hevc_pred_weight_table {
> >>>    * 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
> >>> + * @data_byte_offset: offset (in bytes) 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)
> >>> @@ -356,7 +356,7 @@ struct v4l2_hevc_pred_weight_table {
> >>>    */
> >>>   struct v4l2_ctrl_hevc_slice_params {
> >>>   	__u32	bit_size;
> >>> -	__u32	data_bit_offset;
> >>> +	__u32	data_byte_offset;
> >>>   	__u32	num_entry_point_offsets;
> >>>   	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> >>>   	__u8	nal_unit_type;
> >>> -- 
> >>> 2.32.0
> >>>
> >>>
> >>
> >>
> >
>
Jernej Škrabec June 13, 2022, 6:17 p.m. UTC | #5
Dne nedelja, 12. junij 2022 ob 22:40:29 CEST je Jernej Škrabec napisal(a):
> Dne sreda, 01. junij 2022 ob 18:33:22 CEST je Benjamin Gaignard napisal(a):
> > 
> > Le 01/06/2022 à 18:17, Jernej Škrabec a écrit :
> > > Dne nedelja, 29. maj 2022 ob 08:45:57 CEST je Jernej Škrabec napisal(a):
> > >> Dne petek, 27. maj 2022 ob 16:31:33 CEST je Benjamin Gaignard 
napisal(a):
> > >>> 'F.7.3.6.1 General slice segment header syntax' section of HEVC
> > >>> specification describes that a slice header always end aligned on
> > >>> byte boundary, therefore we only need to provide the data offset in 
> bytes.
> > >>>
> > >>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > >>> ---
> > >>>   Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++--
> > >>>   drivers/staging/media/sunxi/cedrus/cedrus_h265.c          | 2 +-
> > >>>   include/media/hevc-ctrls.h                                | 4 ++--
> > >>>   3 files changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst 
b/
> > >> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > >>> index 48a8825a001b..37079581c661 100644
> > >>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > >>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > >>> @@ -3008,8 +3008,8 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
> > >>>         - ``bit_size``
> > >>>         - Size (in bits) of the current slice data.
> > >>>       * - __u32
> > >>> -      - ``data_bit_offset``
> > >>> -      - Offset (in bits) to the video data in the current slice data.
> > >>> +      - ``data_byte_offset``
> > >>> +      - Offset (in bytes) 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.
> > >>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/
drivers/
> > >> staging/media/sunxi/cedrus/cedrus_h265.c
> > >>> index 411601975124..835454239f73 100644
> > >>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > >>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > >>> @@ -405,7 +405,7 @@ static void cedrus_h265_setup(struct cedrus_ctx 
> *ctx,
> > >>>   	/* Initialize bitstream access. */
> > >>>   	cedrus_write(dev, VE_DEC_H265_TRIGGER,
> > >> VE_DEC_H265_TRIGGER_INIT_SWDEC);
> > >>>   
> > >>> -	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
> > >>> +	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);
> > >> While it's true that actual data starts on 8-bit aligned address, 
Cedrus 
> for
> > >> some reason needs offset which points at the end of the header, before
> > >> alignment. There is very simple way to determine that, but 
unfortunately
> > > this
> > >> means reading source buffer.
> > >>
> > >> In short, above code won't work. I'll provide a fix.
> > > Please include following fix http://ix.io/3Z8x otherwise Cedrus will fail 
> to
> > > decode slice.
> 
> Fluster testing show that this patch isn't completely fine. I also have some 
> other issues, which need control values comparisons. It would be best, if 
you 
> can wait until I finish comparison. Old, hackish HEVC patches have high 
fluster 
> score whereas new, based on stable uAPI, has low and CPU also locks up...

Further testing showed that I make a mistake in ffmpeg code. uAPI and suggested 
changes are fine. It's gtg.

Sorry for the false alarm!
 
Best regards,
Jernej

> 
> > >
> > > Other than fix in previous e-mail and this one, code looks good and I'll 
be
> > > able to add missing functionality to Cedrus without much trouble in 
follow 
> up
> > > series.
> > 
> > Thanks for the patch it will be in version 7.
> > 
> > Regards,
> > Benjamin
> > 
> > >
> > > Best regards,
> > > Jernej
> > >
> > >>>   
> > >>>   	/* Bitstream parameters. */
> > >>>   
> > >>> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> > >>> index 9abca1a75bd4..936ff693967b 100644
> > >>> --- a/include/media/hevc-ctrls.h
> > >>> +++ b/include/media/hevc-ctrls.h
> > >>> @@ -312,7 +312,7 @@ struct v4l2_hevc_pred_weight_table {
> > >>>    * 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
> > >>> + * @data_byte_offset: offset (in bytes) 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)
> > >>> @@ -356,7 +356,7 @@ struct v4l2_hevc_pred_weight_table {
> > >>>    */
> > >>>   struct v4l2_ctrl_hevc_slice_params {
> > >>>   	__u32	bit_size;
> > >>> -	__u32	data_bit_offset;
> > >>> +	__u32	data_byte_offset;
> > >>>   	__u32	num_entry_point_offsets;
> > >>>   	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> > >>>   	__u8	nal_unit_type;
> > >>> -- 
> > >>> 2.32.0
> > >>>
> > >>>
> > >>
> > >>
> > >
> > 
> 
> 
>
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index 48a8825a001b..37079581c661 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -3008,8 +3008,8 @@  enum v4l2_mpeg_video_hevc_size_of_length_field -
       - ``bit_size``
       - Size (in bits) of the current slice data.
     * - __u32
-      - ``data_bit_offset``
-      - Offset (in bits) to the video data in the current slice data.
+      - ``data_byte_offset``
+      - Offset (in bytes) 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.
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 411601975124..835454239f73 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -405,7 +405,7 @@  static void cedrus_h265_setup(struct cedrus_ctx *ctx,
 	/* Initialize bitstream access. */
 	cedrus_write(dev, VE_DEC_H265_TRIGGER, VE_DEC_H265_TRIGGER_INIT_SWDEC);
 
-	cedrus_h265_skip_bits(dev, slice_params->data_bit_offset);
+	cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8);
 
 	/* Bitstream parameters. */
 
diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
index 9abca1a75bd4..936ff693967b 100644
--- a/include/media/hevc-ctrls.h
+++ b/include/media/hevc-ctrls.h
@@ -312,7 +312,7 @@  struct v4l2_hevc_pred_weight_table {
  * 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
+ * @data_byte_offset: offset (in bytes) 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)
@@ -356,7 +356,7 @@  struct v4l2_hevc_pred_weight_table {
  */
 struct v4l2_ctrl_hevc_slice_params {
 	__u32	bit_size;
-	__u32	data_bit_offset;
+	__u32	data_byte_offset;
 	__u32	num_entry_point_offsets;
 	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
 	__u8	nal_unit_type;