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]

 



Le mardi 30 juillet 2019 à 09:21 +0200, Hans Verkuil a écrit :
> On 7/29/19 3:18 PM, Tomasz Figa wrote:
> > On Mon, Jul 29, 2019 at 10:12 PM Paul Kocialkowski
> > <paul.kocialkowski@xxxxxxxxxxx> wrote:
> > > 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.
> 
> I like that name. And this flag is valid for stateful decoders only.

Sorry, I'm not against the name change, but it should be
V4L2_FMT_FLAG_HAS_BYTESTREAM_PARSER (BYTE). Parsers don't support
random bit alignment, so I think usage of bitstream would be miss-
leading. This is playing on words of course, H264 is a bitstream, but
what is passed to the driver is byte aligned, hence a bytestream.

> 
> > > > 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).
> > 
> > My understanding is that a compressed format is any format that
> > doesn't have a directly accessible 2D pixel matrix in its memory
> > representation, so all the bitstream formats should have it set.
> 
> Correct.
> 
> Regards,
> 
> 	Hans
> 
> > Best regards,
> > Tomasz
> > 

Attachment: signature.asc
Description: This is a digitally signed message part


[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