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. > > > 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. Best regards, Tomasz