Date:   Fri, 16 Dec 2022 12:21:19 -0500
From:   Nicolas Dufresne <nicolas@...fresne.ca>
To:     Aakarsh Jain <aakarsh.jain@...sung.com>,
        'Hans Verkuil' <hverkuil-cisco@...all.nl>,
        linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Cc:     m.szyprowski@...sung.com, andrzej.hajda@...el.com,
        mchehab@...nel.org, ezequiel@...guardiasur.com.ar,
        jernej.skrabec@...il.com, benjamin.gaignard@...labora.com,
        stanimir.varbanov@...aro.org, dillon.minfei@...il.com,
        david.plowman@...pberrypi.com, mark.rutland@....com,
        robh+dt@...nel.org, krzk+dt@...nel.org, andi@...zian.org,
        alim.akhtar@...sung.com, aswani.reddy@...sung.com,
        pankaj.dubey@...sung.com, linux-fsd@...la.com, smitha.t@...sung.com
Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.

Le mercredi 14 décembre 2022 à 15:52 +0530, Aakarsh Jain a écrit :
> 
> > -----Original Message-----
> > From: Hans Verkuil [mailto:hverkuil-cisco@...all.nl]
> > Sent: 24 November 2022 16:54
> > To: aakarsh jain <aakarsh.jain@...sung.com>; linux-arm-
> > kernel@...ts.infradead.org; linux-media@...r.kernel.org; linux-
> > kernel@...r.kernel.org; devicetree@...r.kernel.org
> > Cc: m.szyprowski@...sung.com; andrzej.hajda@...el.com;
> > mchehab@...nel.org; ezequiel@...guardiasur.com.ar;
> > jernej.skrabec@...il.com; benjamin.gaignard@...labora.com;
> > stanimir.varbanov@...aro.org; dillon.minfei@...il.com;
> > david.plowman@...pberrypi.com; mark.rutland@....com;
> > robh+dt@...nel.org; krzk+dt@...nel.org; andi@...zian.org;
> > alim.akhtar@...sung.com; aswani.reddy@...sung.com;
> > pankaj.dubey@...sung.com; linux-fsd@...la.com; smitha.t@...sung.com
> > Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> > 
> > On 11/10/2022 14:25, aakarsh jain wrote:
> > > From: Smitha T Murthy <smitha.t@...sung.com>
> > > 
> > > Adds V4l2 controls for VP9 encoder documention.
> > > 
> > > Cc: linux-fsd@...la.com
> > > Signed-off-by: Smitha T Murthy <smitha.t@...sung.com>
> > > Signed-off-by: Aakarsh Jain <aakarsh.jain@...sung.com>
> > > ---
> > >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> > >  1 file changed, 167 insertions(+)
> > > 
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > index 2a165ae063fb..2277d83a7cf0 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> > >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> > >        - Profile 3
> > > 
> > > +VP9 Control Reference
> > 
> > This is wrong. There is a VPX Control Reference section for both VP8 and VP9
> > controls. That's where this should be added. I suspect several of the controls
> > you are adding here already exist, e.g.
> > V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have to be
> > updated to specify that it is for both VP8 and VP9.
> > 
> Since MFC has different profiles, different quantization parameter ranges for both VP8 and VP9. So we can't use same control ID's for both.
> So for example in VP8 with control ID (V4L2_CID_MPEG_VIDEO_VPX_MIN_QP), QP ranges from 0-11 and in VP9 with control ID  (V4L2_CID_CODEC_VP9_MIN_QP) QP ranges from 1-24. So we can't club together into single control.
> 

V4L2_CID_MPEG_VIDEO_VPX_PROFILE has been deprecated, and replace with menu
controls. So we now have a V4L2_CID_MPEG_VIDEO_VP8_PROFILE and a
V4L2_CID_MPEG_VIDEO_VP9_PROFILE as menues. Newly written drivers should use
these. I see that GStreamer notably has never been ported, I'll fix it.

When you implement a driver, the generic uAPI will cover all possible items, as
menus (a integer was an API mistake made in 2011, hence the deprecation). You
driver can then select which menu items it support, and its server at telling
userspace what this HW supports. Though, this should be no problem if you want
to keep the old CID for backward compat, since the range is just totally
undefined there.

For V4L2_CID_MPEG_VIDEO_VPX_MIN_QP (and friends), the doc says "Minimum
quantization parameter for VP8.". A bit strange for a supposedly VPX parameter.
But its defines in the code as "VPX Minimum QP Value". Clearly something to be
fixed. There is no VP9 encoder drivers yet in mainline.

Though, the range for these controls is driver defined. In Venus, for VP8:


        v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
                V4L2_CID_MPEG_VIDEO_VPX_MIN_QP, 1, 128, 1, 1);

It seems to be 1 to 128. While in MFC, it oddly 1 to 11:


        {
                .id = V4L2_CID_MPEG_VIDEO_VPX_MIN_QP,
                .type = V4L2_CTRL_TYPE_INTEGER,
                .minimum = 0,
                .maximum = 11,
                .step = 1,
                .default_value = 0,
        },

While I'm not a huge fan of this, since we all know QP does not scale linearly,
this is how it is, and this is kind of part of the kernel API now. So userspace
must ask the driver what is the QP range, and adapt. And in your case, you
should have no issue adding VP9 encoder with a 1 to 24 range (even if this is a
bit odd and hw specific).

Nicolas


> > > +---------------------
> > > +
> > > +The VP9 controls include controls for encoding parameters of VP9
> > > +video codec.
> > > +
> > > +.. _vp9-control-id:
> > > +
> > > +VP9 Control IDs
> > > +
> > >  .. _v4l2-mpeg-video-vp9-profile:
> > > 
> > >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> > >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> > >        - Level 6.2
> > > 
> > > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> > 
> > If you do need to add new controls, then please use the same
> > MPEG_VIDEO_ prefix.
> > It's a bit ugly and historical, but let's keep it consistent with the others.
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > > +    Quantization parameter for an I frame for VP9. Valid range: from 1 to
> > 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > > +    Quantization parameter for an P frame for VP9. Valid range: from 1 to
> > 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > > +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> > > +    Recommended range for MFC is from 230 to 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > > +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> > > +    Recommended range for MFC is from 1 to 24.
> > > +
> > > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > > +    Indicates the number of evenly spaced subintervals, called ticks, within
> > > +    one second. This is a 16 bit unsigned integer and has a maximum value
> > up to
> > > +    0xffff and a minimum value of 1.
> > > +
> > > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > > +
> > > +.. _v4l2-vp9-golden-frame-sel:
> > > +
> > > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_golden_framesel -
> > > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> > > +    Possible values are:
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > > +      - Use the (n-2)th frame as a golden frame, current frame index being
> > > +        'n'.
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > +      - Use the previous specific frame indicated by
> > > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > > +        golden frame.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > > +    Allows host to specify the quantization parameter values for each
> > > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> > > +    value to 1 enables setting of the QP values for the layers.
> > > +
> > > +.. _v4l2-vp9-ref-number-of-pframes:
> > > +
> > > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > > +    Number of reference pictures for encoding P frames.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > > +      - Indicates one reference frame, last encoded frame will be searched.
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > +      - Indicates 2 reference frames, last encoded frame and golden frame
> > > +        will be searched.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > > +    Indicates the number of hierarchial coding layer.
> > > +    In normal encoding (non-hierarchial coding), it should be zero.
> > > +    VP9 has upto 3 layer of encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > > +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 0.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 1.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 2.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +.. _v4l2-vp9-max-partition-depth:
> > > +
> > > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_num_partitions -
> > > +    Indicate maximum coding unit depth.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > > +      - No coding unit partition depth.
> > > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > > +      - Allows one coding unit partition depth.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > > +    Zero indicates enable intra NxN PU split.
> > > +    One indicates disable intra NxN PU split.
> > > +
> > > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > > +    One indicates disable IVF format.
> > > +
> > > 
> > >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> > > 
> > ==========================================================
> > =
> 
> 

Powered by blists - more mailing lists