Re: [PATCHv4 1/2] media: docs-rst: Document memory-to-memory video decoder interface

[Hans Verkuil]

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