On Fri, Dec 11, 2020 at 5:27 AM Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: > > Le jeudi 10 décembre 2020 à 13:20 +0900, Tomasz Figa a écrit : > > On Thu, Dec 10, 2020 at 1:59 AM Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: > > > > > > 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@xxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxx> > > > > > > wrote: > > > > > > > > > > > > > > Hi Hiro, > > > > > > > > > > > > > > On Fri, Nov 13, 2020 at 6:04 PM Hirokazu Honda <hiroh@xxxxxxxxxxxx> > > > > > > > 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. > > > > > > > Note that this thread is about encoders. > > Sorry about that, but does it makes any difference ? The encoders for H.264 and > HEVC needs to reorder from display to encoded order. That means it will hold on > bitstream buffers. Also, it's an M2M V4L2 device, so it has queues, and you'd > have to do more work in userspace if you had two code path to drain the > asynchronous queues. > A lot of the hardware encoders implement only the basic codec features, e.g. they would only encode in-order (no B-frames), have at most 2 reference frames, etc. In that case, it's as simple as waiting for dequeuing a CAPTURE buffer with a timestamp matching the OUTPUT buffer being waited on. > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > >
