On 01/04/2021 17:59, Benjamin Gaignard wrote:
> > The HEVC HANTRO driver needs to know the number of bits to skip at
> > the beginning of the slice header.
> > That is a hardware specific requirement so create a dedicated control
> > that this purpose.
that -> for
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> > ---
> > version 5:
> > - Be even more verbose in control documentation.
> > - Do not create class for the control.
> > version 4:
> > - The control is now an integer which is enough to provide the numbers
> > of bits to skip.
> > version 3:
> > - Fix typo in field name
> >
> > .../userspace-api/media/drivers/hantro.rst | 14 ++++++++++++++
> > .../userspace-api/media/drivers/index.rst | 1 +
> > include/uapi/linux/v4l2-controls.h | 13 +++++++++++++
> > 3 files changed, 28 insertions(+)
> > create mode 100644
Documentation/userspace-api/media/drivers/hantro.rst
> >
> > diff --git a/Documentation/userspace-api/media/drivers/hantro.rst
b/Documentation/userspace-api/media/drivers/hantro.rst
> > new file mode 100644
> > index 000000000000..78dcd2a44a03
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/drivers/hantro.rst
> > @@ -0,0 +1,14 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +Hantro video decoder driver
> > +===========================
> > +
> > +The Hantro video decoder driver implements the following
driver-specific controls:
> > +
> > +``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)``
> > + Specifies to Hantro HEVC video decoder driver the number of
data (in bits) to
> > + skip in the slice segment header.
> > + If non-IDR, the bits to be skipped go from syntax element
"pic_output_flag"
> > + to before syntax element "slice_temporal_mvp_enabled_flag".
> > + If IDR, the skipped bits are just "pic_output_flag"
> > + (separate_colour_plane_flag is not supported).
Since the driver (and esp. the HEVC API) is still in staging, I prefer
that this
control is defined in include/media/hevc-ctrls.h instead of in
v4l2-controls.h.
Also add a note to the documentation:
.. note::
This control is not yet part of the public kernel API and
it is expected to change.
> > diff --git a/Documentation/userspace-api/media/drivers/index.rst
b/Documentation/userspace-api/media/drivers/index.rst
> > index 1a9038f5f9fa..12e3c512d718 100644
> > --- a/Documentation/userspace-api/media/drivers/index.rst
> > +++ b/Documentation/userspace-api/media/drivers/index.rst
> > @@ -33,6 +33,7 @@ For more details see the file COPYING in the
source distribution of Linux.
> >
> > ccs
> > cx2341x-uapi
> > + hantro
> > imx-uapi
> > max2175
> > meye-uapi
> > diff --git a/include/uapi/linux/v4l2-controls.h
b/include/uapi/linux/v4l2-controls.h
> > index f3376aafea65..1dfb874b6272 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -869,6 +869,19 @@ enum v4l2_mpeg_mfc51_video_force_frame_type {
> > #define V4L2_CID_MPEG_MFC51_VIDEO_H264_ADAPTIVE_RC_STATIC
(V4L2_CID_CODEC_MFC51_BASE+53)
> > #define V4L2_CID_MPEG_MFC51_VIDEO_H264_NUM_REF_PIC_FOR_P
(V4L2_CID_CODEC_MFC51_BASE+54)
> >
> > +/* MPEG-class control IDs specific to the Hantro driver as
defined by V4L2 */
> > +#define V4L2_CID_CODEC_HANTRO_BASE (V4L2_CTRL_CLASS_CODEC | 0x1200)
Right now the stateless HEVC controls are part of the codec class. Once they
are finalized we will move them all over to the stateless codec class.
So I think
keeping this control in the codec class is correct, and when HEVC support is
finalized then this hantro specific control can be moved to the
stateless codec
class as well.
That also means that the note added to the documentation above is
correct: at
that point the control really will change.
Regards,
Hans
> > +/*
> > + * V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP -
> > + * the number of data (in bits) to skip in the
> > + * slice segment header.
> > + * If non-IDR, the bits to be skipped go from syntax element
"pic_output_flag"
> > + * to before syntax element "slice_temporal_mvp_enabled_flag".
> > + * If IDR, the skipped bits are just "pic_output_flag"
> > + * (separate_colour_plane_flag is not supported).
> > + */
> > +#define V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP
(V4L2_CID_CODEC_HANTRO_BASE + 0)
> > +
> > /* Camera class control IDs */
> >
> > #define V4L2_CID_CAMERA_CLASS_BASE (V4L2_CTRL_CLASS_CAMERA | 0x900)
> >
