On 20/05/2020 22:32, Nicolas Dufresne wrote: > Le mercredi 20 mai 2020 à 12:01 +0200, Hans Verkuil a écrit : >> From: Tomasz Figa <tfiga@xxxxxxxxxxxx> >> >> Due to complexity of the video encoding process, the V4L2 drivers of >> stateful encoder hardware require specific sequences of V4L2 API calls >> to be followed. These include capability enumeration, initialization, >> encoding, encode parameters change, drain and reset. >> >> 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 encoder part of >> the Codec API. >> >> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >> --- >> .../userspace-api/media/v4l/dev-encoder.rst | 727 ++++++++++++++++++ >> .../userspace-api/media/v4l/dev-mem2mem.rst | 1 + >> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 5 + >> .../userspace-api/media/v4l/v4l2.rst | 2 + >> .../media/v4l/vidioc-encoder-cmd.rst | 51 +- >> 5 files changed, 766 insertions(+), 20 deletions(-) >> create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst >> >> diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst <snip> >> +5. **Optional** Set the coded frame interval on the ``CAPTURE`` queue via >> + :c:func:`VIDIOC_S_PARM`. This is only necessary if the coded frame >> + interval is different from the raw frame interval, which is typically >> + the case for off-line encoding. >> + >> + * ** Required fields:** >> + >> + ``type`` >> + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``CAPTURE``. >> + >> + ``parm.capture`` >> + set all fields except ``parm.capture.timeperframe`` to 0. >> + >> + ``parm.capture.timeperframe`` >> + the desired coded frame interval; the encoder may adjust it to >> + match hardware requirements. >> + >> + * **Return fields:** >> + >> + ``parm.capture.timeperframe`` >> + the adjusted frame interval. >> + >> + .. important:: >> + >> + Changing the ``CAPTURE`` frame interval sets the framerate for the >> + coded video. It does *not* set the rate at which buffers arrive on the >> + ``CAPTURE`` queue, that depends on how fast the encoder is and how >> + fast raw frames are queued on the ``OUTPUT`` queue. >> + >> + .. important:: >> + >> + ``timeperframe`` deals with *frames*, not fields. So for interlaced >> + formats this is the time per two fields, since a frame consists of >> + a top and a bottom field. >> + >> + .. note:: >> + >> + Not all drivers support this functionality, in that case just set >> + the desired coded frame interval for the ``OUTPUT`` queue. > > There is a slight contorsion in the resulting user-space API. When I > read this, the logical thing to do for live streams would be to just > set the OUTPUT and the driver will take care of CAPTURE for me. > > While if I want to do offline, I don't know if this is supported or > not. So the flow would be a bit special: > > S_PARM(OUTPUT) with coded video frame rate > S_PARM(CAPTURE) width coded video > if ^ worked: > S_PARM(OUTPUT) with fastest rate possible > > Ideally I would have preferred if there was a more straight forward way > to configure offline encoding for fastest performance with specific > coded framerate. I don't think it's a blocker though, performance is > not critical at all here. Maybe I'm missing something that allow to > check if this is supported or not without trying it ? Good point. I considered adding a flag for the v4l2_fmtdesc struct that reports whether you can set the capture framerate independently from the OUTPUT framerate. Perhaps V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL? I actually think it would be best if that's added. It is not enough to rely on whether S_PARM(CAPTURE) works to determine this feature since at least one encoder drivers supports both OUTPUT and CAPTURE with S_PARM, but CAPTURE does the same as OUTPUT, so that would be a red herring. I'll add this flag for v3. Regards, Hans
