[FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: fix draining process (dequeue without input)

[wm4]

On Tue, 26 Sep 2017 16:22:23 -0700
Jorge Ramirez-Ortiz <jorge.ramirez-ortiz at linaro.org> wrote:

> Stopping the codec when no more input is available causes captured
> buffers that might be ready to be dequeued to be invalidated.
> 
> This commit follows the V4L2 API more closely:
> 1. on the last valid input buffer, it sets the V4L2_BUF_FLAG_LAST.
> 2. ffmpeg then will continue dequeuing captured buffers until EPIPE is
> raised by the driver (EOF condition)
> 
> This also has the advantage of making the timeout on dequeuing capture
> buffers while draining unnecessary.
> ---
>  libavcodec/v4l2_context.c | 162 ++++++++++++++++++----------------------------
>  1 file changed, 64 insertions(+), 98 deletions(-)

I can't really comment on the logic of this code. So LGTM, just some
minor comments/questions.

> -    /* 0. handle errors */
> +    /* handle errors */

(Apparently) unrelated cosmetic changes should usually be in separate
patches, but that's probably not worth the trouble in this case.

> +    if (!frame) {
> +        /* flag that we are draining */
> +        ctx_to_m2mctx(ctx)->draining = 1;
> +
> +        /* send EOS */
> +        avbuf->buf.m.planes[0].bytesused = 1;
> +        avbuf->flags |= V4L2_BUF_FLAG_LAST;
> +    } else {
> +        ret = ff_v4l2_buffer_avframe_to_buf(frame, avbuf);
> +        if (ret)
> +            return ret;
> +    }

Wouldn't it be better to switch the if/else bodies and avoid the
negation in the if condition?

> -    ret = ff_v4l2_buffer_avpkt_to_buf(pkt, avbuf);
> -    if (ret)
> -        return ret;
> +    if (!pkt->size) {
> +        /* flag that we are draining */
> +        ctx_to_m2mctx(ctx)->draining = 1;
> +
> +        /* send EOS */
> +        avbuf->buf.m.planes[0].bytesused = 1;
> +        avbuf->flags |= V4L2_BUF_FLAG_LAST;

What is going on here? Dummy buffer of size 1 to send the flag?