Le dimanche 16 mai 2021 à 20:04 -0300, Ezequiel Garcia a écrit : > Hi Hans, > > On Thu, 2021-05-06 at 14:50 +0200, Hans Verkuil wrote: > > On 05/05/2021 17:20, Benjamin Gaignard wrote: > > > > > > 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.gaign...@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. > > > > But since hantro is very common, any userland decoder will need to > > calculate this anyway. > > So perhaps it is better to have this as part of the decode_params? > > > > I'd like to know what others think about this. > > > > As you know, I'm not a fan of carrying these "unstable" APIs around. > I know it's better than nothing, but I feel they create the illusion > of the interface being supported in mainline. Since it's unstable, > it's difficult for applications to adopt them. > > As Nicolas mentioned, this means neither FFmpeg nor GStreamer will adopt > these APIs, which worries me, as that means we lose two major user bases. > > My personal take from this, is that we need to find ways to stabilize > our stateless codec APIs in less time and perhaps with less effort. > > IMO, a less stiff interface could help us here, and that's why I think > having hardware-specific controls can be useful. Hardware designers > can be so creative :) > > I'm not against introducing this specific parameter in > v4l2_ctrl_hevc_codec_params, arguing that Hantro is widely used, > but I'd like us to be open to hardware-specific controls as a way > to extend the APIs seamlessly. > > Applications won't have to _know_ what hardware they are running on, > they can just use VIDIOC_QUERYCTRL to find out which controls are needed.
Can you extend on this, perhaps we need an RFC for this specific mechanism. I don't immediatly see how I could enumerate controls and figure-out which one are needed. Perhaps we need to add new control flags for mandatory control ? This way userspace could detect unsupported HW if it finds a mandatory control that it does not know about ? > > Thanks, > Ezequiel > _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
