LinuxLists.cc - [RFC 0/7] media: Clean and make H264 stateless uAPI public

On Thu, 2020-06-25 at 11:01 -0400, Nicolas Dufresne wrote:
> Le mardi 23 juin 2020 à 15:28 -0300, Ezequiel Garcia a écrit :
> > Slice header syntax element 'first_mb_in_slice' can point
> > to the last macroblock, currently the field can only reference
> > 65536 macroblocks which is insufficient for 8K videos.
> >
> > DPB entry PicNum maximum value is 2*MaxFrameNum for interlaced
> > content (field_pic_flag=1).
> >
> > Therefore, increase 'first_mb_in_slice' and 'pic_num'.
> >
> > The v4l2_h264_dpb_entry struct needs to be padded to avoid a hole,
> > which will be useful to allow future uAPI extensions.
> >
> > Note that v4l2_ctrl_h264_slice_params struct will be modified
> > in a follow-up commit, and so we defer its 64-bit padding.
>
> This patch includes two changes, with two distinct rationale. Please
> split in two, I would also add reference to the spec in the commit
> messages.

OK.

> The explanation is also insufficient. Need to mention that
> macro-blocks are 16x16, and the bounds for synthetic value
> MaxFrameNum (derived from bitstream value) has not be mention either.
>

OK, will do.

Thanks!
Ezequiel

> > Signed-off-by: Ezequiel Garcia <[email protected]>
> > ---
> > .../userspace-api/media/v4l/ext-ctrls-codec.rst | 7 +++++--
> > drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++++++
> > include/media/h264-ctrls.h | 6 ++++--
> > include/media/v4l2-h264.h | 2 +-
> > 4 files changed, 19 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 0808a36777b6..e3b5a28fb965 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1772,7 +1772,7 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u32
> > - ``header_bit_size``
> > -
> > - * - __u16
> > + * - __u32
> > - ``first_mb_in_slice``
> > -
> > * - __u8
> > @@ -2046,7 +2046,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u16
> > - ``frame_num``
> > -
> > - * - __u16
> > + * - __u8
> > + - ``reserved[6]``
> > + - Applications and drivers must set this to zero.
> > + * - __u32
> > - ``pic_num``
> > -
> > * - __s32
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 6abd023f10c7..a751c14f9c22 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -1734,6 +1734,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
> > struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
> > + struct v4l2_ctrl_h264_decode_params *p_h264_dec_params;
> > struct v4l2_ctrl_hevc_sps *p_hevc_sps;
> > struct v4l2_ctrl_hevc_pps *p_hevc_pps;
> > struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
> > @@ -1808,6 +1809,14 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > }
> > break;
> > case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
> > + p_h264_dec_params = p;
> > +
> > + for (i = 0; i < V4L2_H264_NUM_DPB_ENTRIES; i++) {
> > + struct v4l2_h264_dpb_entry *dpb_entry =
> > + &p_h264_dec_params->dpb[i];
> > +
> > + zero_reserved(*dpb_entry);
> > + }
> > break;
> >
> > case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
> > diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> > index c6cbf178c1c9..a938d16b901c 100644
> > --- a/include/media/h264-ctrls.h
> > +++ b/include/media/h264-ctrls.h
> > @@ -161,7 +161,8 @@ struct v4l2_ctrl_h264_slice_params {
> > /* Offset in bits to slice_data() from the beginning of this slice. */
> > __u32 header_bit_size;
> >
> > - __u16 first_mb_in_slice;
> > + __u32 first_mb_in_slice;
> > +
> > __u8 slice_type;
> > __u8 pic_parameter_set_id;
> > __u8 colour_plane_id;
> > @@ -208,7 +209,8 @@ struct v4l2_ctrl_h264_slice_params {
> > struct v4l2_h264_dpb_entry {
> > __u64 reference_ts;
> > __u16 frame_num;
> > - __u16 pic_num;
> > + __u8 reserved[6];
> > + __u32 pic_num;
> > /* Note that field is indicated by v4l2_buffer.field */
> > __s32 top_field_order_cnt;
> > __s32 bottom_field_order_cnt;
> > diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
> > index bc9ebb560ccf..1a5f26fc2a9a 100644
> > --- a/include/media/v4l2-h264.h
> > +++ b/include/media/v4l2-h264.h
> > @@ -33,7 +33,7 @@ struct v4l2_h264_reflist_builder {
> > struct {
> > s32 pic_order_count;
> > int frame_num;
> > - u16 pic_num;
> > + u32 pic_num;
> > u16 longterm : 1;
> > } refs[V4L2_H264_NUM_DPB_ENTRIES];
> > s32 cur_pic_order_count;

2020-06-25 15:32:15

by Tomasz Figa

[permalink] [raw]

Subject: Re: [RFC 2/7] fixup! media: uapi: h264: update reference lists

On Tue, Jun 23, 2020 at 8:29 PM Ezequiel Garcia <[email protected]> wrote:
>
> Align v4l2_h264_reference to 32-bits, giving some room
> for future extensions.
>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> .../userspace-api/media/v4l/ext-ctrls-codec.rst | 3 +++
> drivers/media/v4l2-core/v4l2-ctrls.c | 16 ++++++++++++++++
> include/media/h264-ctrls.h | 7 +++++--
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 6c36d298db20..7af12447a5b0 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1943,6 +1943,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - __u8
> - ``index``
> -
> + * - __u32
> + - ``reserved``
> + - Applications and drivers must set this to zero.

__u16?

Best regards,
Tomasz

2020-06-25 18:22:32

by Ezequiel Garcia

[permalink] [raw]

Subject: Re: [RFC 2/7] fixup! media: uapi: h264: update reference lists

On Thu, 2020-06-25 at 17:30 +0200, Tomasz Figa wrote:
> On Tue, Jun 23, 2020 at 8:29 PM Ezequiel Garcia <[email protected]> wrote:
> > Align v4l2_h264_reference to 32-bits, giving some room
> > for future extensions.
> >
> > Signed-off-by: Ezequiel Garcia <[email protected]>
> > ---
> > .../userspace-api/media/v4l/ext-ctrls-codec.rst | 3 +++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 16 ++++++++++++++++
> > include/media/h264-ctrls.h | 7 +++++--
> > 3 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 6c36d298db20..7af12447a5b0 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1943,6 +1943,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u8
> > - ``index``
> > -
> > + * - __u32
> > + - ``reserved``
> > + - Applications and drivers must set this to zero.
>
> __u16?
>
>

Indeed :)

Thanks,
Ezequiel

2020-06-25 18:31:27

by Ezequiel Garcia

[permalink] [raw]

Subject: Re: [RFC 2/7] fixup! media: uapi: h264: update reference lists

(Adding Jernej, seems I haven't Cced him!)

On Thu, 2020-06-25 at 10:53 -0400, Nicolas Dufresne wrote:
> Le mardi 23 juin 2020 à 15:28 -0300, Ezequiel Garcia a écrit :
> > Align v4l2_h264_reference to 32-bits, giving some room
> > for future extensions.
> >
> > Signed-off-by: Ezequiel Garcia <[email protected]>
> > ---
> > .../userspace-api/media/v4l/ext-ctrls-codec.rst | 3 +++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 16 ++++++++++++++++
> > include/media/h264-ctrls.h | 7 +++++--
> > 3 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 6c36d298db20..7af12447a5b0 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1943,6 +1943,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u8
> > - ``index``
> > -
> > + * - __u32
> > + - ``reserved``
> > + - Applications and drivers must set this to zero.
>
> Is that really appropriate ? There was some effort to keep the controls
> small. Also, as these are fixed size, they could be extended with
> supplementary C arrays of the same size, set in separate control.
>

That's a very valid concern.

Currently, each of these v4l2_h264_reference take 64 bytes
(32 x 2 bytes). So it's 128 bytes of references.

Having the reserved field means 256 bytes instead.

Without the reserved field v4l2_ctrl_h264_slice_params is 936 bytes,
the majority of the space taken by v4l2_h264_weight_factors.
So, the reserved field accounts for a small increment there.

Another option would be to split the v4l2_h264_reference lists
to its own control.

However, given the above, it would have more impact to consider
splitting v4l2_h264_pred_weight_table. This table
being used or not depends on the bitstream, so there might be
some value here.

What do you think?

> Also, H264 HW is unlikely to evolve, and that covers what DXVA2 and
> VAAPI covers already. So it is quite unlikely to ever have to be
> extended.
>

That may be true indeed. Perhaps we can then agree that references
only need an index and a flag.

Thanks!
Ezequiel

> >
> > .. _h264_reference_flags:
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 3f3fbcd60cc6..6abd023f10c7 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -1721,6 +1721,8 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> >
> > #define zero_padding(s) \
> > memset(&(s).padding, 0, sizeof((s).padding))
> > +#define zero_reserved(s) \
> > + memset(&(s).reserved, 0, sizeof((s).reserved))
> >
> > /*
> > * Compound controls validation requires setting unused fields/flags to zero
> > @@ -1731,6 +1733,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > {
> > struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
> > + struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
> > struct v4l2_ctrl_hevc_sps *p_hevc_sps;
> > struct v4l2_ctrl_hevc_pps *p_hevc_pps;
> > struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
> > @@ -1790,7 +1793,20 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > case V4L2_CTRL_TYPE_H264_SPS:
> > case V4L2_CTRL_TYPE_H264_PPS:
> > case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
> > + break;
> > case V4L2_CTRL_TYPE_H264_SLICE_PARAMS:
> > + p_h264_slice_params = p;
> > +
> > + for (i = 0; i < V4L2_H264_REF_PIC_LIST_LEN; i++) {
> > + struct v4l2_h264_reference *ref0 =
> > + &p_h264_slice_params->ref_pic_list0[i];
> > + struct v4l2_h264_reference *ref1 =
> > + &p_h264_slice_params->ref_pic_list1[i];
> > +
> > + zero_reserved(*ref0);
> > + zero_reserved(*ref1);
> > + }
> > + break;
> > case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
> > break;
> >
> > diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> > index 9b1cbc9bc38e..c6cbf178c1c9 100644
> > --- a/include/media/h264-ctrls.h
> > +++ b/include/media/h264-ctrls.h
> > @@ -19,6 +19,8 @@
> > */
> > #define V4L2_H264_NUM_DPB_ENTRIES 16
> >
> > +#define V4L2_H264_REF_PIC_LIST_LEN 32
> > +
> > /* Our pixel format isn't stable at the moment */
> > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */
> >
> > @@ -146,6 +148,7 @@ struct v4l2_h264_pred_weight_table {
> > struct v4l2_h264_reference {
> > __u8 flags;
> > __u8 index;
> > + __u16 reserved;
> > };
> >
> > struct v4l2_ctrl_h264_slice_params {
> > @@ -190,8 +193,8 @@ struct v4l2_ctrl_h264_slice_params {
> > * Entries on each list are indices into
> > * v4l2_ctrl_h264_decode_params.dpb[].
> > */
> > - struct v4l2_h264_reference ref_pic_list0[32];
> > - struct v4l2_h264_reference ref_pic_list1[32];
> > + struct v4l2_h264_reference ref_pic_list0[V4L2_H264_REF_PIC_LIST_LEN];
> > + struct v4l2_h264_reference ref_pic_list1[V4L2_H264_REF_PIC_LIST_LEN];
> >
> > __u32 flags;
> > };

2020-06-25 18:32:45

On Thu, 2020-06-25 at 11:05 -0400, Nicolas Dufresne wrote:
> Le mardi 23 juin 2020 à 15:28 -0300, Ezequiel Garcia a écrit :
> > The H.264 specification requires in its "Slice header semantics"
> > section that the following values shall be the same in all slice headers:
> >
> > pic_parameter_set_id
> > frame_num
> > field_pic_flag
> > bottom_field_flag
> > idr_pic_id
> > pic_order_cnt_lsb
> > delta_pic_order_cnt_bottom
> > delta_pic_order_cnt[ 0 ]
> > delta_pic_order_cnt[ 1 ]
> > sp_for_switch_flag
> > slice_group_change_cycle
> >
> > and can therefore be moved to the per-frame decode parameters control.
> >
> > Field 'pic_parameter_set_id' is simply removed in this case,
> > because the PPS control must currently contain the active PPS.
> >
> > Syntax elements dec_ref_pic_marking() and those related
> > to pic order count, remain invariant as well, and therefore,
> > the fields dec_ref_pic_marking_bit_size and pic_order_cnt_bit_size
> > are also common to all slices.
> >
> > Signed-off-by: Ezequiel Garcia <[email protected]>
>
> This looks good to me. Perhaps drivers that don't need the slice_params
> (hantro) should simply not implement it ?
>

Yes, but let's keep that out of this series.

> Assuming this is/will be thoroughly tested by porting userspace code:
>

Of course!
> Reviewed-by: Nicolas Dufresne <[email protected]>
>

Thanks,
Ezequiel

> > ---
> > .../media/v4l/ext-ctrls-codec.rst | 84 +++++++++----------
> > drivers/media/v4l2-core/v4l2-ctrls.c | 1 +
> > drivers/media/v4l2-core/v4l2-h264.c | 8 +-
> > .../staging/media/hantro/hantro_g1_h264_dec.c | 21 +++--
> > drivers/staging/media/hantro/hantro_h264.c | 3 +-
> > drivers/staging/media/rkvdec/rkvdec-h264.c | 6 +-
> > .../staging/media/sunxi/cedrus/cedrus_h264.c | 9 +-
> > include/media/h264-ctrls.h | 43 +++++-----
> > include/media/v4l2-h264.h | 1 -
> > 9 files changed, 86 insertions(+), 90 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 2c682f81aaad..24b30ff37d9a 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1781,44 +1781,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u8
> > - ``slice_type``
> > -
> > - * - __u8
> > - - ``pic_parameter_set_id``
> > - -
> > * - __u8
> > - ``colour_plane_id``
> > -
> > * - __u8
> > - ``redundant_pic_cnt``
> > -
> > - * - __u16
> > - - ``frame_num``
> > - -
> > - * - __u16
> > - - ``idr_pic_id``
> > - -
> > - * - __u16
> > - - ``pic_order_cnt_lsb``
> > - -
> > - * - __s32
> > - - ``delta_pic_order_cnt_bottom``
> > - -
> > - * - __s32
> > - - ``delta_pic_order_cnt0``
> > - -
> > - * - __s32
> > - - ``delta_pic_order_cnt1``
> > - -
> > - * - struct :c:type:`v4l2_h264_pred_weight_table`
> > - - ``pred_weight_table``
> > - -
> > - * - __u32
> > - - ``dec_ref_pic_marking_bit_size``
> > - - Size in bits of the dec_ref_pic_marking() syntax element.
> > - * - __u32
> > - - ``pic_order_cnt_bit_size``
> > - - Combined size in bits of the picture order count related syntax
> > - elements: pic_order_cnt_lsb, delta_pic_order_cnt_bottom,
> > - delta_pic_order_cnt0, and delta_pic_order_cnt1.
> > * - __u8
> > - ``cabac_init_idc``
> > -
> > @@ -1845,8 +1813,11 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > - ``num_ref_idx_l1_active_minus1``
> > - If num_ref_idx_active_override_flag is not set, this field must be
> > set to the value of num_ref_idx_l1_default_active_minus1.
> > - * - __u32
> > - - ``slice_group_change_cycle``
> > + * - __u8
> > + - ``reserved[5]``
> > + - Applications and drivers must set this to zero.
> > + * - struct :c:type:`v4l2_h264_pred_weight_table`
> > + - ``pred_weight_table``
> > -
> > * - struct :c:type:`v4l2_h264_reference`
> > - ``ref_pic_list0[32]``
> > @@ -1869,17 +1840,11 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > :stub-columns: 0
> > :widths: 1 1 2
> >
> > - * - ``V4L2_H264_SLICE_FLAG_FIELD_PIC``
> > - - 0x00000001
> > - -
> > - * - ``V4L2_H264_SLICE_FLAG_BOTTOM_FIELD``
> > - - 0x00000002
> > - -
> > * - ``V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED``
> > - - 0x00000004
> > + - 0x00000001
> > -
> > * - ``V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH``
> > - - 0x00000008
> > + - 0x00000002
> > -
> >
> > ``Prediction Weight Table``
> > @@ -2011,6 +1976,35 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __s32
> > - ``bottom_field_order_cnt``
> > - Picture Order Count for the coded bottom field
> > + * - __u16
> > + - ``frame_num``
> > + -
> > + * - __u16
> > + - ``idr_pic_id``
> > + -
> > + * - __u16
> > + - ``pic_order_cnt_lsb``
> > + -
> > + * - __s32
> > + - ``delta_pic_order_cnt_bottom``
> > + -
> > + * - __s32
> > + - ``delta_pic_order_cnt0``
> > + -
> > + * - __s32
> > + - ``delta_pic_order_cnt1``
> > + -
> > + * - __u32
> > + - ``dec_ref_pic_marking_bit_size``
> > + - Size in bits of the dec_ref_pic_marking() syntax element.
> > + * - __u32
> > + - ``pic_order_cnt_bit_size``
> > + - Combined size in bits of the picture order count related syntax
> > + elements: pic_order_cnt_lsb, delta_pic_order_cnt_bottom,
> > + delta_pic_order_cnt0, and delta_pic_order_cnt1.
> > + * - __u32
> > + - ``slice_group_change_cycle``
> > + -
> > * - __u32
> > - ``flags``
> > - See :ref:`Decode Parameters Flags <h264_decode_params_flags>`
> > @@ -2029,6 +2023,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - ``V4L2_H264_DECODE_PARAM_FLAG_IDR_PIC``
> > - 0x00000001
> > - That picture is an IDR picture
> > + * - ``V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC``
> > + - 0x00000002
> > + -
> > + * - ``V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD``
> > + - 0x00000004
> > + -
> >
> > .. c:type:: v4l2_h264_dpb_entry
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 003333b6c7f7..a21f96c7d077 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -1812,6 +1812,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > zero_reserved(*ref0);
> > zero_reserved(*ref1);
> > }
> > + zero_reserved(*p_h264_slice_params);
> > break;
> > case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
> > p_h264_dec_params = p;
> > diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
> > index edf6225f0522..791d24e1b72f 100644
> > --- a/drivers/media/v4l2-core/v4l2-h264.c
> > +++ b/drivers/media/v4l2-core/v4l2-h264.c
> > @@ -18,14 +18,12 @@
> > *
> > * @b: the builder context to initialize
> > * @dec_params: decode parameters control
> > - * @slice_params: first slice parameters control
> > * @sps: SPS control
> > * @dpb: DPB to use when creating the reference list
> > */
> > void
> > v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> > const struct v4l2_ctrl_h264_decode_params *dec_params,
> > - const struct v4l2_ctrl_h264_slice_params *slice_params,
> > const struct v4l2_ctrl_h264_sps *sps,
> > const struct v4l2_h264_dpb_entry dpb[V4L2_H264_NUM_DPB_ENTRIES])
> > {
> > @@ -33,13 +31,13 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> > unsigned int i;
> >
> > max_frame_num = 1 << (sps->log2_max_frame_num_minus4 + 4);
> > - cur_frame_num = slice_params->frame_num;
> > + cur_frame_num = dec_params->frame_num;
> >
> > memset(b, 0, sizeof(*b));
> > - if (!(slice_params->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC))
> > + if (!(dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC))
> > b->cur_pic_order_count = min(dec_params->bottom_field_order_cnt,
> > dec_params->top_field_order_cnt);
> > - else if (slice_params->flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD)
> > + else if (dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
> > b->cur_pic_order_count = dec_params->bottom_field_order_cnt;
> > else
> > b->cur_pic_order_count = dec_params->top_field_order_cnt;
> > diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > index 424c648ce9fc..f9839e9c6da5 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > @@ -23,7 +23,6 @@ static void set_params(struct hantro_ctx *ctx)
> > {
> > const struct hantro_h264_dec_ctrls *ctrls = &ctx->h264_dec.ctrls;
> > const struct v4l2_ctrl_h264_decode_params *dec_param = ctrls->decode;
> > - const struct v4l2_ctrl_h264_slice_params *slices = ctrls->slices;
> > const struct v4l2_ctrl_h264_sps *sps = ctrls->sps;
> > const struct v4l2_ctrl_h264_pps *pps = ctrls->pps;
> > struct vb2_v4l2_buffer *src_buf = hantro_get_src_buf(ctx);
> > @@ -42,11 +41,11 @@ static void set_params(struct hantro_ctx *ctx)
> >
> > if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY) &&
> > (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD ||
> > - slices[0].flags & V4L2_H264_SLICE_FLAG_FIELD_PIC))
> > + dec_param->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC))
> > reg |= G1_REG_DEC_CTRL0_PIC_INTERLACE_E;
> > - if (slices[0].flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + if (dec_param->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC)
> > reg |= G1_REG_DEC_CTRL0_PIC_FIELDMODE_E;
> > - if (!(slices[0].flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD))
> > + if (!(dec_param->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD))
> > reg |= G1_REG_DEC_CTRL0_PIC_TOPFIELD_E;
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
> >
> > @@ -75,7 +74,7 @@ static void set_params(struct hantro_ctx *ctx)
> >
> > /* Decoder control register 4. */
> > reg = G1_REG_DEC_CTRL4_FRAMENUM_LEN(sps->log2_max_frame_num_minus4 + 4) |
> > - G1_REG_DEC_CTRL4_FRAMENUM(slices[0].frame_num) |
> > + G1_REG_DEC_CTRL4_FRAMENUM(dec_param->frame_num) |
> > G1_REG_DEC_CTRL4_WEIGHT_BIPR_IDC(pps->weighted_bipred_idc);
> > if (pps->flags & V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE)
> > reg |= G1_REG_DEC_CTRL4_CABAC_E;
> > @@ -88,8 +87,8 @@ static void set_params(struct hantro_ctx *ctx)
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL4);
> >
> > /* Decoder control register 5. */
> > - reg = G1_REG_DEC_CTRL5_REFPIC_MK_LEN(slices[0].dec_ref_pic_marking_bit_size) |
> > - G1_REG_DEC_CTRL5_IDR_PIC_ID(slices[0].idr_pic_id);
> > + reg = G1_REG_DEC_CTRL5_REFPIC_MK_LEN(dec_param->dec_ref_pic_marking_bit_size) |
> > + G1_REG_DEC_CTRL5_IDR_PIC_ID(dec_param->idr_pic_id);
> > if (pps->flags & V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED)
> > reg |= G1_REG_DEC_CTRL5_CONST_INTRA_E;
> > if (pps->flags & V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT)
> > @@ -103,10 +102,10 @@ static void set_params(struct hantro_ctx *ctx)
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL5);
> >
> > /* Decoder control register 6. */
> > - reg = G1_REG_DEC_CTRL6_PPS_ID(slices[0].pic_parameter_set_id) |
> > + reg = G1_REG_DEC_CTRL6_PPS_ID(pps->pic_parameter_set_id) |
> > G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(pps->num_ref_idx_l0_default_active_minus1 + 1) |
> > G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(pps->num_ref_idx_l1_default_active_minus1 + 1) |
> > - G1_REG_DEC_CTRL6_POC_LENGTH(slices[0].pic_order_cnt_bit_size);
> > + G1_REG_DEC_CTRL6_POC_LENGTH(dec_param->pic_order_cnt_bit_size);
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL6);
> >
> > /* Error concealment register. */
> > @@ -246,7 +245,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> > /* Destination (decoded frame) buffer. */
> > dst_dma = hantro_get_dec_buf_addr(ctx, &dst_buf->vb2_buf);
> > /* Adjust dma addr to start at second line for bottom field */
> > - if (ctrls->slices[0].flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD)
> > + if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
> > offset = ALIGN(ctx->src_fmt.width, MB_DIM);
> > vdpu_write_relaxed(vpu, dst_dma + offset, G1_REG_ADDR_DST);
> >
> > @@ -265,7 +264,7 @@ static void set_buffers(struct hantro_ctx *ctx)
> > * DMV buffer is split in two for field encoded frames,
> > * adjust offset for bottom field
> > */
> > - if (ctrls->slices[0].flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD)
> > + if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
> > offset += 32 * MB_WIDTH(ctx->src_fmt.width) *
> > MB_HEIGHT(ctx->src_fmt.height);
> > vdpu_write_relaxed(vpu, dst_dma + offset, G1_REG_ADDR_DIR_MV);
> > diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
> > index d561f125085a..9890ae7bf3bc 100644
> > --- a/drivers/staging/media/hantro/hantro_h264.c
> > +++ b/drivers/staging/media/hantro/hantro_h264.c
> > @@ -374,8 +374,7 @@ int hantro_h264_dec_prepare_run(struct hantro_ctx *ctx)
> >
> > /* Build the P/B{0,1} ref lists. */
> > v4l2_h264_init_reflist_builder(&reflist_builder, ctrls->decode,
> > - &ctrls->slices[0], ctrls->sps,
> > - ctx->h264_dec.dpb);
> > + ctrls->sps, ctx->h264_dec.dpb);
> > v4l2_h264_build_p_ref_list(&reflist_builder, h264_ctx->reflists.p);
> > v4l2_h264_build_b_ref_lists(&reflist_builder, h264_ctx->reflists.b0,
> > h264_ctx->reflists.b1);
> > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > index 7b66e2743a4f..0f5d519528d8 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > @@ -730,7 +730,6 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> > struct rkvdec_h264_run *run)
> > {
> > const struct v4l2_ctrl_h264_decode_params *dec_params = run->decode_params;
> > - const struct v4l2_ctrl_h264_slice_params *sl_params = &run->slices_params[0];
> > const struct v4l2_h264_dpb_entry *dpb = dec_params->dpb;
> > struct rkvdec_h264_ctx *h264_ctx = ctx->priv;
> > const struct v4l2_ctrl_h264_sps *sps = run->sps;
> > @@ -754,7 +753,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> > continue;
> >
> > if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM ||
> > - dpb[i].frame_num < sl_params->frame_num) {
> > + dpb[i].frame_num < dec_params->frame_num) {
> > p[i] = dpb[i].frame_num;
> > continue;
> > }
> > @@ -1093,8 +1092,7 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)
> >
> > /* Build the P/B{0,1} ref lists. */
> > v4l2_h264_init_reflist_builder(&reflist_builder, run.decode_params,
> > - &run.slices_params[0], run.sps,
> > - run.decode_params->dpb);
> > + run.sps, run.decode_params->dpb);
> > h264_ctx->reflists.num_valid = reflist_builder.num_valid;
> > v4l2_h264_build_p_ref_list(&reflist_builder, h264_ctx->reflists.p);
> > v4l2_h264_build_b_ref_lists(&reflist_builder, h264_ctx->reflists.b0,
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > index cce527bbdf86..e7387315253d 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > @@ -95,7 +95,6 @@ static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
> > {
> > struct cedrus_h264_sram_ref_pic pic_list[CEDRUS_H264_FRAME_NUM];
> > const struct v4l2_ctrl_h264_decode_params *decode = run->h264.decode_params;
> > - const struct v4l2_ctrl_h264_slice_params *slice = run->h264.slice_params;
> > const struct v4l2_ctrl_h264_sps *sps = run->h264.sps;
> > struct vb2_queue *cap_q;
> > struct cedrus_buffer *output_buf;
> > @@ -144,7 +143,7 @@ static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
> > output_buf = vb2_to_cedrus_buffer(&run->dst->vb2_buf);
> > output_buf->codec.h264.position = position;
> >
> > - if (slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + if (decode->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC)
> > output_buf->codec.h264.pic_type = CEDRUS_H264_PIC_TYPE_FIELD;
> > else if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
> > output_buf->codec.h264.pic_type = CEDRUS_H264_PIC_TYPE_MBAFF;
> > @@ -414,7 +413,7 @@ static void cedrus_set_params(struct cedrus_ctx *ctx,
> > reg |= VE_H264_SPS_DIRECT_8X8_INFERENCE;
> > cedrus_write(dev, VE_H264_SPS, reg);
> >
> > - mbaff_pic = !(slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC) &&
> > + mbaff_pic = !(decode->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC) &&
> > (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD);
> > pic_width_in_mbs = sps->pic_width_in_mbs_minus1 + 1;
> >
> > @@ -428,9 +427,9 @@ static void cedrus_set_params(struct cedrus_ctx *ctx,
> > reg |= slice->cabac_init_idc & 0x3;
> > if (ctx->fh.m2m_ctx->new_frame)
> > reg |= VE_H264_SHS_FIRST_SLICE_IN_PIC;
> > - if (slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + if (decode->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC)
> > reg |= VE_H264_SHS_FIELD_PIC;
> > - if (slice->flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD)
> > + if (decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
> > reg |= VE_H264_SHS_BOTTOM_FIELD;
> > if (slice->flags & V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED)
> > reg |= VE_H264_SHS_DIRECT_SPATIAL_MV_PRED;
> > diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> > index 4119dc4486f3..6446ec9f283d 100644
> > --- a/include/media/h264-ctrls.h
> > +++ b/include/media/h264-ctrls.h
> > @@ -138,10 +138,8 @@ struct v4l2_h264_pred_weight_table {
> > #define V4L2_H264_SLICE_TYPE_SP 3
> > #define V4L2_H264_SLICE_TYPE_SI 4
> >
> > -#define V4L2_H264_SLICE_FLAG_FIELD_PIC 0x01
> > -#define V4L2_H264_SLICE_FLAG_BOTTOM_FIELD 0x02
> > -#define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04
> > -#define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
> > +#define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x01
> > +#define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x02
> >
> > #define V4L2_H264_REFERENCE_FLAG_TOP_FIELD 0x01
> > #define V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD 0x02
> > @@ -165,22 +163,8 @@ struct v4l2_ctrl_h264_slice_params {
> > __u32 first_mb_in_slice;
> >
> > __u8 slice_type;
> > - __u8 pic_parameter_set_id;
> > __u8 colour_plane_id;
> > __u8 redundant_pic_cnt;
> > - __u16 frame_num;
> > - __u16 idr_pic_id;
> > - __u16 pic_order_cnt_lsb;
> > - __s32 delta_pic_order_cnt_bottom;
> > - __s32 delta_pic_order_cnt0;
> > - __s32 delta_pic_order_cnt1;
> > -
> > - struct v4l2_h264_pred_weight_table pred_weight_table;
> > - /* Size in bits of dec_ref_pic_marking() syntax element. */
> > - __u32 dec_ref_pic_marking_bit_size;
> > - /* Size in bits of pic order count syntax. */
> > - __u32 pic_order_cnt_bit_size;
> > -
> > __u8 cabac_init_idc;
> > __s8 slice_qp_delta;
> > __s8 slice_qs_delta;
> > @@ -189,7 +173,9 @@ struct v4l2_ctrl_h264_slice_params {
> > __s8 slice_beta_offset_div2;
> > __u8 num_ref_idx_l0_active_minus1;
> > __u8 num_ref_idx_l1_active_minus1;
> > - __u32 slice_group_change_cycle;
> > + __u8 reserved[5];
> > +
> > + struct v4l2_h264_pred_weight_table pred_weight_table;
> >
> > /*
> > * Entries on each list are indices into
> > @@ -218,7 +204,9 @@ struct v4l2_h264_dpb_entry {
> > __u32 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */
> > };
> >
> > -#define V4L2_H264_DECODE_PARAM_FLAG_IDR_PIC 0x01
> > +#define V4L2_H264_DECODE_PARAM_FLAG_IDR_PIC 0x01
> > +#define V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC 0x02
> > +#define V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD 0x04
> >
> > struct v4l2_ctrl_h264_decode_params {
> > struct v4l2_h264_dpb_entry dpb[V4L2_H264_NUM_DPB_ENTRIES];
> > @@ -226,6 +214,21 @@ struct v4l2_ctrl_h264_decode_params {
> > __u16 nal_ref_idc;
> > __s32 top_field_order_cnt;
> > __s32 bottom_field_order_cnt;
> > +
> > + __u16 frame_num;
> > + __u16 idr_pic_id;
> > + __u16 reserved;
> > +
> > + __u16 pic_order_cnt_lsb;
> > + __s32 delta_pic_order_cnt_bottom;
> > + __s32 delta_pic_order_cnt0;
> > + __s32 delta_pic_order_cnt1;
> > + /* Size in bits of dec_ref_pic_marking() syntax element. */
> > + __u32 dec_ref_pic_marking_bit_size;
> > + /* Size in bits of pic order count syntax. */
> > + __u32 pic_order_cnt_bit_size;
> > + __u32 slice_group_change_cycle;
> > +
> > __u32 flags; /* V4L2_H264_DECODE_PARAM_FLAG_* */
> > };
> >
> > diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
> > index 1a5f26fc2a9a..f08ba181263d 100644
> > --- a/include/media/v4l2-h264.h
> > +++ b/include/media/v4l2-h264.h
> > @@ -44,7 +44,6 @@ struct v4l2_h264_reflist_builder {
> > void
> > v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> > const struct v4l2_ctrl_h264_decode_params *dec_params,
> > - const struct v4l2_ctrl_h264_slice_params *slice_params,
> > const struct v4l2_ctrl_h264_sps *sps,
> > const struct v4l2_h264_dpb_entry dpb[V4L2_H264_NUM_DPB_ENTRIES]);
> >