Re: [PATCH 02/14] videodev2.h: add V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun 28 Jul 19, 23:05, Tomasz Figa wrote:
> On Sat, Jul 27, 2019 at 6:37 PM Paul Kocialkowski
> <paul.kocialkowski@xxxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Wed 24 Jul 19, 13:05, Hans Verkuil wrote:
> > > Add an enum_fmt format flag to specifically tag coded formats where
> > > full bitstream parsing is supported by the device.
> > >
> > > Some stateful decoders are capable of fully parsing a bitstream,
> > > but others require that userspace pre-parses the bitstream into
> > > frames or fields (see the corresponding pixelformat descriptions
> > > for details).
> > >
> > > If this flag is set, then this pre-parsing step is not required
> > > (but still possible, of course).
> > >
> > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> > > ---
> > >  Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++++++++
> > >  Documentation/media/videodev2.h.rst.exceptions   | 1 +
> > >  include/uapi/linux/videodev2.h                   | 5 +++--
> > >  3 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > index 822d6730e7d2..4e24e671f32e 100644
> > > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > > @@ -127,6 +127,14 @@ one until ``EINVAL`` is returned.
> > >        - This format is not native to the device but emulated through
> > >       software (usually libv4l2), where possible try to use a native
> > >       format instead for better performance.
> > > +    * - ``V4L2_FMT_FLAG_HAS_BITSTREAM_PARSER``
> > > +      - 0x0004
> > > +      - The hardware decoder for this compressed bitstream format (aka coded
> > > +     format) is capable of parsing the bitstream. Applications do not
> > > +     need to parse the bitstream themselves to find the boundaries between
> > > +     frames/fields. This flag can only be used in combination with the
> > > +     ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed
> > > +     formats only.
> >
> > Should this flag be set for stateless codecs as well? It seems a bit over-kill
> > for this case. I am not sure whether "compressed bitstream format" clearly only
> > covers the formats used by stateful decoders and not the ones for stateless
> > decoders.
> 
> I'd suggest using a different name for the flag, because "bitstream
> parser" is actually one of the core differences between stateful and
> stateless. All stateful decoders have bitstream parsers, the only
> difference between the implementations is the unit on which the parser
> operates, i.e. full stream, frame, NALU.
> 
> Perhaps V4L2_FMT_FLAG_CONTINUOUS_BITSTREAM (as opposed to discrete,
> framed/sliced chunks)?

Sure, that seems like a more explicit name regarding what it's supposed to
describe in my opinion.

> Regardless of that, it doesn't make sense for a stateless decoder to
> set this flag anyway, because the userspace needs to parse the whole
> stream anyway and the whole stateless API is based on the assumption
> that the userspace splits the bitstream into frames (or slices).

Indeed, I agree that it doesn't make sense, but I thought that the name of the
flag could be confusing. Since there is no direct equivalency between
"stateless" and "doesn't parse the bitstream" (as we've seen with the rockchip
decoder needing to parse the slice header on its own), that could have been
ambiguous. I think the name you're suggesting mostly solves this concern.

I'm still a bit unsure about what "compressed formats" entails or not, so it
could be good to explicitly mention that this applies to stateful decoders only
(but it's just a suggestion, advanced users of the API will probably find it
straightforward).

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux