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. > > > > > > > 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 > > > > > > > > > > > >