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. >> >>> 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 >