On 7/10/19 10:23 AM, Tomasz Figa wrote: > On Wed, Jul 10, 2019 at 5:09 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: >> >> On 7/3/19 6:58 AM, Tomasz Figa wrote: >>> Hi Hans, >>> >>> On Mon, Jun 3, 2019 at 8:28 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: >>>> >>>> From: Tomasz Figa <tfiga@xxxxxxxxxxxx> >>>> >>>> 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> >>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >>>> --- >>>> Documentation/media/uapi/v4l/dev-decoder.rst | 1084 +++++++++++++++++ >>>> Documentation/media/uapi/v4l/dev-mem2mem.rst | 8 +- >>>> Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 5 + >>>> Documentation/media/uapi/v4l/v4l2.rst | 10 +- >>>> .../media/uapi/v4l/vidioc-decoder-cmd.rst | 41 +- >>>> 5 files changed, 1132 insertions(+), 16 deletions(-) >>>> create mode 100644 Documentation/media/uapi/v4l/dev-decoder.rst >>>> >>> >>> Thanks a lot for helping with remaining changes. >>> >>> Just one thing inline our team member found recently. >>> >>> [snip] >>>> +Capture setup >>>> +============= >>>> + >>> [snip] >>>> +4. **Optional.** Set the ``CAPTURE`` format via :c:func:`VIDIOC_S_FMT` on the >>>> + ``CAPTURE`` queue. The client may choose a different format than >>>> + selected/suggested by the decoder in :c:func:`VIDIOC_G_FMT`. >>>> + >>>> + * **Required fields:** >>>> + >>>> + ``type`` >>>> + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``CAPTURE``. >>>> + >>>> + ``pixelformat`` >>>> + a raw pixel format. >>> >>> The client should be able to set the width and height as well. It's a >>> quite frequent case, especially in DMA-buf import mode, that the >>> buffers are actually bigger (e.g. more alignment) than what we could >>> get from the decoder by default. For sane hardware platforms it's >>> reasonable to expect that such bigger buffers could be handled as >>> well, as long as we update the width and height here. >> >> I've added this: >> >> ``width``, ``height`` >> frame buffer resolution of the decoded stream; typically unchanged from >> what was returned with :c:func:`VIDIOC_G_FMT`, but it may be different >> if the hardware supports composition and/or scaling. >> >> Is that what you were looking for? >> > > Not sure if composition is a requirement here, but I guess it depends > on how we define composition. Most of the hardware today at least > support arbitrary strides (+/- some alignment), but still write the > pixels at (0,0)x(w,h). > > In fact, there would be already some composition happening, even > without arbitrary strides, because G_FMT would return values aligned > in some way, but only the visible rectangle would contain meaningful > pixel data. Pretty much all codec drivers can handle composition, and with that I don't mean the macroblock alignment, but that it can compose in, say, a buffer that's twice the width/height than is strictly required. And yes, it is a limited form of composition in that the top left corner typically can't be changed. Regards, Hans