Messages in this thread | | | Subject | Re: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver | From | Benjamin Gaignard <> | Date | Wed, 5 May 2021 17:20:46 +0200 |
| |
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 >> >
| |