lkml.org 
[lkml]   [2021]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver
    From
    Date

    Le 05/05/2021 à 16:55, Hans Verkuil a écrit :
    > On 20/04/2021 14:10, 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
    >> for this purpose.
    >>
    >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
    >> ---
    >> .../userspace-api/media/drivers/hantro.rst | 19 +++++++++++++++++++
    >> .../userspace-api/media/drivers/index.rst | 1 +
    >> include/media/hevc-ctrls.h | 13 +++++++++++++
    >> 3 files changed, 33 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..cd9754b4e005
    >> --- /dev/null
    >> +++ b/Documentation/userspace-api/media/drivers/hantro.rst
    >> @@ -0,0 +1,19 @@
    >> +.. 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).
    > I'm not very keen on this. Without this information the video data cannot be
    > decoded, or will it just be suboptimal?

    Without that information the video can't be decoded.

    >
    > The problem is that a generic decoder would have to know that the HW is a hantro,
    > and then call this control. If they don't (and are testing on non-hantro HW), then
    > it won't work, thus defeating the purpose of the HW independent decoder API.
    >
    > Since hantro is widely used, and if there is no other way to do this beside explitely
    > setting this control, then perhaps this should be part of the standard HEVC API.
    > Non-hantro drivers that do not need this can just skip it.

    Even if I put this parameter in decode_params structure that would means that a generic
    userland decoder will have to know how the compute this value for hantro HW since it
    isn't something that could be done on kernel side.


    Regards,
    Benjamin

    >
    > Regards,
    >
    > Hans
    >
    >> +
    >> +.. 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/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
    >> index 8e0109eea454..b713eeed1915 100644
    >> --- a/include/media/hevc-ctrls.h
    >> +++ b/include/media/hevc-ctrls.h
    >> @@ -224,4 +224,17 @@ struct v4l2_ctrl_hevc_decode_params {
    >> __u64 flags;
    >> };
    >>
    >> +/* MPEG-class control IDs specific to the Hantro driver as defined by V4L2 */
    >> +#define V4L2_CID_CODEC_HANTRO_BASE (V4L2_CTRL_CLASS_CODEC | 0x1200)
    >> +/*
    >> + * 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)
    >> +
    >> #endif
    >>
    >

    \
     
     \ /
      Last update: 2021-05-05 17:21    [W:3.717 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site