On 1/31/19 1:30 PM, Hans Verkuil wrote: > On 1/31/19 11:45 AM, Hans Verkuil wrote: >> On 1/24/19 11:04 AM, Tomasz Figa wrote: >>> Due to complexity of the video decoding process, the V4L2 drivers of >>> stateful decoder hardware require specific sequences of V4L2 API calls >>> to be followed. These include capability enumeration, initialization, >>> decoding, seek, pause, dynamic resolution change, drain and end of >>> stream. >>> >>> Specifics of the above have been discussed during Media Workshops at >>> LinuxCon Europe 2012 in Barcelona and then later Embedded Linux >>> Conference Europe 2014 in Düsseldorf. The de facto Codec API that >>> originated at those events was later implemented by the drivers we already >>> have merged in mainline, such as s5p-mfc or coda. >>> >>> The only thing missing was the real specification included as a part of >>> Linux Media documentation. Fix it now and document the decoder part of >>> the Codec API. >>> >>> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> >>> --- >>> Documentation/media/uapi/v4l/dev-decoder.rst | 1076 +++++++++++++++++ >>> Documentation/media/uapi/v4l/dev-mem2mem.rst | 5 + >>> Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 5 + >>> Documentation/media/uapi/v4l/v4l2.rst | 10 +- >>> .../media/uapi/v4l/vidioc-decoder-cmd.rst | 40 +- >>> Documentation/media/uapi/v4l/vidioc-g-fmt.rst | 14 + >>> 6 files changed, 1135 insertions(+), 15 deletions(-) >>> create mode 100644 Documentation/media/uapi/v4l/dev-decoder.rst >>> >> >> <snip> >> >>> +4. **This step only applies to coded formats that contain resolution information >>> + in the stream.** Continue queuing/dequeuing bitstream buffers to/from the >>> + ``OUTPUT`` queue via :c:func:`VIDIOC_QBUF` and :c:func:`VIDIOC_DQBUF`. The >>> + buffers will be processed and returned to the client in order, until >>> + required metadata to configure the ``CAPTURE`` queue are found. This is >>> + indicated by the decoder sending a ``V4L2_EVENT_SOURCE_CHANGE`` event with >>> + ``V4L2_EVENT_SRC_CH_RESOLUTION`` source change type. >>> + >>> + * It is not an error if the first buffer does not contain enough data for >>> + this to occur. Processing of the buffers will continue as long as more >>> + data is needed. >>> + >>> + * If data in a buffer that triggers the event is required to decode the >>> + first frame, it will not be returned to the client, until the >>> + initialization sequence completes and the frame is decoded. >>> + >>> + * If the client sets width and height of the ``OUTPUT`` format to 0, >>> + calling :c:func:`VIDIOC_G_FMT`, :c:func:`VIDIOC_S_FMT`, >>> + :c:func:`VIDIOC_TRY_FMT` or :c:func:`VIDIOC_REQBUFS` on the ``CAPTURE`` >>> + queue will return the ``-EACCES`` error code, until the decoder >>> + configures ``CAPTURE`` format according to stream metadata. >> >> I think this should also include the G/S_SELECTION ioctls, right? > > I've started work on adding compliance tests for codecs to v4l2-compliance and > I quickly discovered that this 'EACCES' error code is not nice at all. > > The problem is that it is really inconsistent with V4L2 behavior: the basic > rule is that there always is a format defined, i.e. G_FMT will always return > a format. > > Suddenly returning an error is actually quite painful to handle because it is > a weird exception just for the capture queue of a stateful decoder if no > output resolution is known. > > Just writing that sentence is painful. Note that adding EACCES support in vicodec was just as painful as adding v4l2-compliance support for it. So it is also easier to handle this in the driver if you can just set up a default format. Regards, Hans > > Why not just return some default driver defined format? It will automatically > be updated once the decoder parsed the bitstream and knows the new resolution. > > It really is just the same behavior as with a resolution change. > > It is also perfectly fine to request buffers for the capture queue for that > default format. It's pointless, but not a bug. > > Unless I am missing something I strongly recommend changing this behavior. > > Regards, > > Hans >
