On Wed, 2021-05-05 at 16:55 +0200, Hans Verkuil wrote: > 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@xxxxxxxxxxxxx> > > --- > > .../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? > > The problem is that a generic decoder would have to know that the HW is a hantro, Applications can just query which controls are exposed by a video device, and if this control is found, then it means it needs to be set. > 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. > The decision to move it out of the HEVC API is not really to avoid setting it. In the end, most/all applications will end up required to set this
