Re: How to get last frames in encode sequence returned by v4l2 encoder driver without V4L2_ENC_CMD_STOP

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Le mercredi 09 décembre 2020 à 18:51 +0900, Tomasz Figa a écrit :
> On Wed, Dec 9, 2020 at 1:43 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > 
> > Le vendredi 27 novembre 2020 à 03:13 +0900, Hirokazu Honda a écrit :
> > > HI Alexandre,
> > > 
> > > On Tue, Nov 24, 2020 at 7:17 PM Alexandre Courbot <acourbot@chromium.org>
> > > wrote:
> > > > 
> > > > Hi Hiro,
> > > > 
> > > > On Fri, Nov 13, 2020 at 6:04 PM Hirokazu Honda <hiroh@chromium.org>
> > > > wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > According to the official v4l2 encoder driver usage description [1],
> > > > > v4l2 steatful encoder driver doesn't have a guarantee when frames fed
> > > > > to a driver will be returned.
> > > > > To make sure all pending frames are output by the driver, an app must
> > > > > call VIDIOC_ENCODER_CMD with cmd=V4L2_ENC_CMD_STOP.
> > > > > However, it is not mandatory to support the command in the current
> > > > > v4l2 stateful encoder API specification.
> > > > > An app can check it by VIDIOC_TRY_ENCODER_CMD beforehand.
> > > > > My question is when an app has to get all the frames of an encoder
> > > > > sequence, how we can achieve this without V4L2_ENC_CMD_STOP support.
> > > > > This demand is natural and in fact WebCodecs [2] requires this.
> > > > > 
> > > > > I think there are two options,
> > > > > 1.) Ensure that a driver will eventually output frames if it doesn't
> > > > > support V4L2_ENC_CMD_STOP.
> > > > > 2.) Change V4L2_ENC_CMD_STOP support to be mandatory
> > > > 
> > > > Unless I am missing the part of the spec that says the contrary,
> > > > V4L2_ENC_CMD_STOP is part of the encoder specification, and thus is
> > > > mandatory. Some older drivers might not have support for it, in such
> > > > cases the correct course of action would be to fix them.
> > > > 
> > > 
> > > I researched the API documents.
> > > The statement that the support is mandatory to stateful encoders is
> > > added from the latest document v5.9 [1],
> > > It states optional in the API document of v4.19 and v5.8.
> > > Hence my question is obsolete.
> > > 
> > > [1]
> > > https://www.kernel.org/doc/html/v5.9/userspace-api/media/v4l/vidioc-encoder-cmd.html
> > > [2]
> > > https://www.kernel.org/doc/html/v4.19/media/uapi/v4l/vidioc-encoder-cmd.html
> > > [3]
> > > https://www.kernel.org/doc/html/v5.8/userspace-api/media/v4l/vidioc-encoder-cmd.html?highlight=v4l2_enc_cmd_stop
> > 
> > In historical drivers (Samsung MFC and perhaps Venus ?) an empty payload
> > buffer
> > was used to signal draining. This approach was never documented and had
> > issues.
> > It is still supported by MFC driver so that older Chromium / Android code
> > don't
> > fail on it (even though I doubt it has ever been retested since).
> > 
> 
> FWIW, Chromium has not been relying on this for a long time already.
> 
> That said, I think the question here is about a different class of
> devices. I can imagine some encoders just simply always consuming the
> input buffers as they come and produce corresponding bitstream buffers
> as soon as possible, in a 1:1 relationship. In that case, there would
> be no need for any explicit drain, because one can track which buffers
> came already and infer whether the encoding completed for the source
> buffer of interest.

I believe we decided to make this mantory to all stateless decoder (it makes
userspace life easier). But JPEG decoders predates the spec, so I suspect a JPEG
decoding userspace should behave as you describe to actually work with existing
drivers. I would rather encourage having CMD_STOP in all stateless decoder, even
JPEG.

> 
> Best regards,
> Tomasz
> 
> > > 
> > > Best Regards,
> > > -Hiro
> > > > > 
> > > > > Any comments are appreciated.
> > > > > Thanks so much in advance.
> > > > > 
> > > > > [1]
> > > > > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-encoder.html#drain
> > > > > [2] https://web.dev/webcodecs/
> > > > > 
> > > > > Sincerely,
> > > > > -Hiro
> > 
> >