Re: [PATCH v5 01/15] media: uapi: HEVC: Add v4l2_ctrl_hevc_ext_sps_[ls]t_rps controls

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 11479 bytes --]

Hi,

Le mardi 16 décembre 2025 à 15:48 -0500, Detlev Casanova a écrit :
> Hi Hans,
> 
> On 10/24/25 03:55, Hans Verkuil wrote:
> > On 23/10/2025 23:42, Detlev Casanova wrote:
> > > Some hardware (e.g.: Rockchip's rk3588 hevc decoder) need the
> > > long and short term reference information for HEVC decoding.
> > > 
> > > Add controls to provide it as the raw data form the stream.
> > > 
> > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > ---
> > >   .../media/v4l/ext-ctrls-codec-stateless.rst   | 114 ++++++++++++++++++
> > >   .../media/v4l/videodev2.h.rst.exceptions      |   2 +
> > >   .../media/v4l/vidioc-queryctrl.rst            |  12 ++
> > >   3 files changed, 128 insertions(+)
> > > 
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> > > index 497ae74379f6..7b18e12fafbb 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> > > @@ -2959,6 +2959,120 @@ This structure contains all loop filter related parameters. See sections
> > >         - 0x00000004
> > >         -
> > >   
> > > +``V4L2_CID_STATELESS_HEVC_EXT_SPS_LT_RPS (struct)``
> > > +    Specifies the list of Long-Term reference sets parameters from the SPS.
> > > +    These parameters are defined according to :ref:`hevc`.
> > > +    They are described in section 7.4.3.2.1 "General sequence parameter set
> > > +    RBSP semantics" of the specification.
> > > +    This control is a dynamically sized 1-dimensional array,
> > > +    V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag must be set when using it.
> > I'd drop this last sentence. It suggests that the application has to set it,
> > but is it set by the kernel.
> > 
> > Same for the next control below.
> > 
> > I noticed that this is probably a copy-and-paste from the V4L2_CID_STATELESS_HEVC_SLICE_PARAMS
> > documentation: can you post a separate patch removing that same line from the description of
> > that control? I think it is a confusing line, and we missed it during the original review
> > of V4L2_CID_STATELESS_HEVC_SLICE_PARAMS.
> > 
> > I have another question about these two controls: currently dynamic controls have always at
> > least one element. I have been working on supporting zero-sized dynamic controls. Would
> > that make sense here? Or will these two controls - if present - always have one element or
> > more?
> > 
> > If zero-length dynamic array controls are useful here, then I can clean up my patches and
> > post them so you can use it.
> 
> I guess that could happen depending on what userspace does, but the size 
> of the array is determined by num_long_term_ref_pics_sps and 
> num_short_term_ref_pic_sets from v4l2_ctrl_hevc_sps respectively.
> 
> If that value is 0, there is no point in setting the (respective) 
> control, except for simplifying userspace code I suppose.
> 
> So 0-length support is avoidable.

With our rule of keeping controls as consistent as possible, we'd need a zero
length array here since key frame have zero long and short references.

Basically, we should be able to clear that control when
long_term_ref_pics_present_flag is not set, or num_short_term_ref_pic_sets is
zero.

Feel free to send an update anyway, since I don't expect Hans to be able to
provide this code now, would have been better to reply earlier. In fact, I agree
we can probably move on without the zero-length immediately added and add it
later.

Before going that route, how big are these two arrays ? Short term can be 64
ref, long term 32, how big in bytes that is ? Was is worth goign with dynamic
array ? Its not like slices, where we can have 600 or more.

Nicolas

> 
> Detlev.
> 
> > Regards,
> > 
> > 	Hans
> > 
> > > +
> > > +.. c:type:: v4l2_ctrl_hevc_ext_sps_lt_rps
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table:: struct v4l2_ctrl_hevc_ext_sps_lt_rps
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 1 2
> > > +
> > > +    * - __u16
> > > +      - ``lt_ref_pic_poc_lsb_sps``
> > > +      - Long term reference picture order count as described in section 7.4.3.2.1
> > > +        "General sequence parameter set RBSP semantics" of the specification.
> > > +    * - __u8
> > > +      - ``flags``
> > > +      - See :ref:`Extended Long-Term RPS Flags <hevc_ext_sps_lt_rps_flags>`
> > > +
> > > +.. _hevc_ext_sps_lt_rps_flags:
> > > +
> > > +``Extended SPS Long-Term RPS Flags``
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 1 2
> > > +
> > > +    * - ``V4L2_HEVC_EXT_SPS_LT_RPS_FLAG_USED_LT``
> > > +      - 0x00000001
> > > +      - Specifies if the long-term reference picture is used 7.4.3.2.1 "General sequence parameter
> > > +        set RBSP semantics" of the specification.
> > > +
> > > +``V4L2_CID_STATELESS_HEVC_EXT_SPS_ST_RPS (struct)``
> > > +    Specifies the list of Short-Term reference sets parameters from the SPS.
> > > +    These parameters are defined according to :ref:`hevc`.
> > > +    They are described in section 7.4.8 "Short-term reference picture set
> > > +    semantics" of the specification.
> > > +    This control is a dynamically sized 1-dimensional array,
> > > +    V4L2_CTRL_FLAG_DYNAMIC_ARRAY flag must be set when using it.
> > > +
> > > +.. c:type:: v4l2_ctrl_hevc_ext_sps_st_rps
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table:: struct v4l2_ctrl_hevc_ext_sps_st_rps
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 1 2
> > > +
> > > +    * - __u8
> > > +      - ``delta_idx_minus1``
> > > +      - Specifies the delta compare to the index. See details in section 7.4.8 "Short-term
> > > +        reference picture set semantics" of the specification.
> > > +    * - __u8
> > > +      - ``delta_rps_sign``
> > > +      - Sign of the delta as specified in section 7.4.8 "Short-term reference picture set
> > > +        semantics" of the specification.
> > > +    * - __u16
> > > +      - ``abs_delta_rps_minus1``
> > > +      - Absolute delta RPS as specified in section 7.4.8 "Short-term reference picture set
> > > +        semantics" of the specification.
> > > +    * - __u8
> > > +      - ``num_negative_pics``
> > > +      - Number of short-term RPS entries that have picture order count values less than the
> > > +        picture order count value of the current picture.
> > > +    * - __u8
> > > +      - ``num_positive_pics``
> > > +      - Number of short-term RPS entries that have picture order count values greater than the
> > > +        picture order count value of the current picture.
> > > +    * - __u32
> > > +      - ``used_by_curr_pic``
> > > +      - Bit i specifies if short-term RPS i is used by the current picture.
> > > +    * - __u32
> > > +      - ``use_delta_flag``
> > > +      - Bit i specifies if short-term RPS i is included in the short-term RPS entries.
> > > +    * - __u16
> > > +      - ``delta_poc_s0_minus1[16]``
> > > +      - Specifies the negative picture order count delta for the i-th entry in the short-term RPS.
> > > +        See details in section 7.4.8 "Short-term reference picture set semantics" of the
> > > +        specification.
> > > +    * - __u16
> > > +      - ``delta_poc_s1_minus1[16]``
> > > +      - Specifies the positive picture order count delta for the i-th entry in the short-term RPS.
> > > +        See details in section 7.4.8 "Short-term reference picture set semantics" of the
> > > +        specification.
> > > +    * - __u8
> > > +      - ``flags``
> > > +      - See :ref:`Extended Short-Term RPS Flags <hevc_ext_sps_st_rps_flags>`
> > > +
> > > +.. _hevc_ext_sps_st_rps_flags:
> > > +
> > > +``Extended SPS Short-Term RPS Flags``
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 1 2
> > > +
> > > +    * - ``V4L2_HEVC_EXT_SPS_ST_RPS_FLAG_INTER_REF_PIC_SET_PRED``
> > > +      - 0x00000001
> > > +      - Specifies if the short-term RPS is predicted from another short term RPS. See details in
> > > +        section 7.4.8 "Short-term reference picture set semantics" of the specification.
> > > +
> > >   .. _v4l2-codec-stateless-av1:
> > >   
> > >   ``V4L2_CID_STATELESS_AV1_SEQUENCE (struct)``
> > > diff --git a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> > > index c41693115db6..6182b4e2d2ee 100644
> > > --- a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> > > +++ b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> > > @@ -150,6 +150,8 @@ replace symbol V4L2_CTRL_TYPE_H264_SCALING_MATRIX :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_H264_PRED_WEIGHTS :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_H264_SLICE_PARAMS :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_H264_DECODE_PARAMS :c:type:`V4L.v4l2_ctrl_type`
> > > +replace symbol V4L2_CTRL_TYPE_HEVC_EXT_SPS_ST_RPS :c:type:`V4L.v4l2_ctrl_type`
> > > +replace symbol V4L2_CTRL_TYPE_HEVC_EXT_SPS_LT_RPS :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`V4L.v4l2_ctrl_type`
> > >   replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`V4L.v4l2_ctrl_type`
> > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > > index c8baa9430c14..82c8b52e771c 100644
> > > --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > > +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > > @@ -531,6 +531,18 @@ See also the examples in :ref:`control`.
> > >         - n/a
> > >         - A struct :c:type:`v4l2_ctrl_hevc_decode_params`, containing HEVC
> > >   	decoding parameters for stateless video decoders.
> > > +    * - ``V4L2_CTRL_TYPE_HEVC_EXT_SPS_LT_RPS``
> > > +      - n/a
> > > +      - n/a
> > > +      - n/a
> > > +      - A struct :c:type:`v4l2_ctrl_hevc_ext_sps_lt_rps`, containing HEVC
> > > +	extended Long-Term RPS for stateless video decoders.
> > > +    * - ``V4L2_CTRL_TYPE_HEVC_EXT_SPS_ST_RPS``
> > > +      - n/a
> > > +      - n/a
> > > +      - n/a
> > > +      - A struct :c:type:`v4l2_ctrl_hevc_ext_sps_st_rps`, containing HEVC
> > > +	extended Short-Term RPS for stateless video decoders.
> > >       * - ``V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR``
> > >         - n/a
> > >         - n/a

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]