diff mbox series

[FFmpeg-devel,1/2] avcodec/v4l2_context: Drop empty packet while draining

Message ID 20200429212942.28797-1-andriy.gelman@gmail.com
State Accepted
Commit e3b49aaa4eed7955e243b110e1209960ba5aaf74
Headers show
Series [FFmpeg-devel,1/2] avcodec/v4l2_context: Drop empty packet while draining | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andriy Gelman April 29, 2020, 9:29 p.m. UTC 
From: Andriy Gelman <andriy.gelman@gmail.com>

v4l2_m2m devices may send an empty packet/frame while draining
to indicate that all capture buffers have been flushed.

Currently, the empty packet/frame is not handled correctly:
When encoding, the empty packet is forwarded to the muxer, usually
creating warnings.
When decoding, a reference to the memory is created anyway.. Since in
the past this memory contained a decoded frame, it results in an extra
frame being decoded.

This commit discards the empty packet/frame.

References:
linux/Documentation/media/uapi/v4l/dev-decoder.rst:

    "The last buffer may be empty (with :c:type:`v4l2_buffer` bytesused = 0)
     and in that case it must be ignored by the client, as it does not
     contain a decoded frame."

linux/Documentation/media/uapi/media/v4l/vidioc-encoder-cmd.rst:

    "...This buffer may be empty, indicated by the
     driver setting the ``bytesused`` field to 0."

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavcodec/v4l2_context.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Ming Qian April 30, 2020, 1:39 a.m. UTC | #1 
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> v4l2_m2m devices may send an empty packet/frame while draining to indicate
> that all capture buffers have been flushed.
> 
> Currently, the empty packet/frame is not handled correctly:
> When encoding, the empty packet is forwarded to the muxer, usually creating
> warnings.
> When decoding, a reference to the memory is created anyway.. Since in the
> past this memory contained a decoded frame, it results in an extra frame being
> decoded.
> 
> This commit discards the empty packet/frame.
> 
> References:
> linux/Documentation/media/uapi/v4l/dev-decoder.rst:
> 
>     "The last buffer may be empty (with :c:type:`v4l2_buffer` bytesused = 0)
>      and in that case it must be ignored by the client, as it does not
>      contain a decoded frame."
> 
> linux/Documentation/media/uapi/media/v4l/vidioc-encoder-cmd.rst:
> 
>     "...This buffer may be empty, indicated by the
>      driver setting the ``bytesused`` field to 0."
> 
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>  libavcodec/v4l2_context.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c index
> 6c2db5c849..f0ecc18ebd 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -393,6 +393,15 @@ dequeue:
>              return NULL;
>          }
> 
> +        if (ctx_to_m2mctx(ctx)->draining
> && !V4L2_TYPE_IS_OUTPUT(ctx->type)) {
> +            int bytesused = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ?
> +                            buf.m.planes[0].bytesused : buf.bytesused;
> +            if (bytesused == 0) {
> +                ctx->done = 1;
> +                return NULL;
> +            }
> +        }
> +
>          avbuf = &ctx->buffers[buf.index];
>          avbuf->status = V4L2BUF_AVAILABLE;
>          avbuf->buf = buf;
> --
> 2.25.1
> 

Lgtm
Andriy Gelman May 5, 2020, 9:29 p.m. UTC | #2 
On Thu, 30. Apr 01:39, Ming Qian wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > v4l2_m2m devices may send an empty packet/frame while draining to indicate
> > that all capture buffers have been flushed.
> > 
> > Currently, the empty packet/frame is not handled correctly:
> > When encoding, the empty packet is forwarded to the muxer, usually creating
> > warnings.
> > When decoding, a reference to the memory is created anyway.. Since in the
> > past this memory contained a decoded frame, it results in an extra frame being
> > decoded.
> > 
> > This commit discards the empty packet/frame.
> > 
> > References:
> > linux/Documentation/media/uapi/v4l/dev-decoder.rst:
> > 
> >     "The last buffer may be empty (with :c:type:`v4l2_buffer` bytesused = 0)
> >      and in that case it must be ignored by the client, as it does not
> >      contain a decoded frame."
> > 
> > linux/Documentation/media/uapi/media/v4l/vidioc-encoder-cmd.rst:
> > 
> >     "...This buffer may be empty, indicated by the
> >      driver setting the ``bytesused`` field to 0."
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> >  libavcodec/v4l2_context.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c index
> > 6c2db5c849..f0ecc18ebd 100644
> > --- a/libavcodec/v4l2_context.c
> > +++ b/libavcodec/v4l2_context.c
> > @@ -393,6 +393,15 @@ dequeue:
> >              return NULL;
> >          }
> > 
> > +        if (ctx_to_m2mctx(ctx)->draining
> > && !V4L2_TYPE_IS_OUTPUT(ctx->type)) {
> > +            int bytesused = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ?
> > +                            buf.m.planes[0].bytesused : buf.bytesused;
> > +            if (bytesused == 0) {
> > +                ctx->done = 1;
> > +                return NULL;
> > +            }
> > +        }
> > +
> >          avbuf = &ctx->buffers[buf.index];
> >          avbuf->status = V4L2BUF_AVAILABLE;
> >          avbuf->buf = buf;
> > --
> > 2.25.1
> > 

> 
> Lgtm
> 

Thanks. 
Will apply both patches on Friday if no one objects.
Andriy Gelman May 9, 2020, 11:45 p.m. UTC | #3 
On Tue, 05. May 17:29, Andriy Gelman wrote:
> On Thu, 30. Apr 01:39, Ming Qian wrote:
> > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > > 
> > > v4l2_m2m devices may send an empty packet/frame while draining to indicate
> > > that all capture buffers have been flushed.
> > > 
> > > Currently, the empty packet/frame is not handled correctly:
> > > When encoding, the empty packet is forwarded to the muxer, usually creating
> > > warnings.
> > > When decoding, a reference to the memory is created anyway.. Since in the
> > > past this memory contained a decoded frame, it results in an extra frame being
> > > decoded.
> > > 
> > > This commit discards the empty packet/frame.
> > > 
> > > References:
> > > linux/Documentation/media/uapi/v4l/dev-decoder.rst:
> > > 
> > >     "The last buffer may be empty (with :c:type:`v4l2_buffer` bytesused = 0)
> > >      and in that case it must be ignored by the client, as it does not
> > >      contain a decoded frame."
> > > 
> > > linux/Documentation/media/uapi/media/v4l/vidioc-encoder-cmd.rst:
> > > 
> > >     "...This buffer may be empty, indicated by the
> > >      driver setting the ``bytesused`` field to 0."
> > > 
> > > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > > ---
> > >  libavcodec/v4l2_context.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c index
> > > 6c2db5c849..f0ecc18ebd 100644
> > > --- a/libavcodec/v4l2_context.c
> > > +++ b/libavcodec/v4l2_context.c
> > > @@ -393,6 +393,15 @@ dequeue:
> > >              return NULL;
> > >          }
> > > 
> > > +        if (ctx_to_m2mctx(ctx)->draining
> > > && !V4L2_TYPE_IS_OUTPUT(ctx->type)) {
> > > +            int bytesused = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ?
> > > +                            buf.m.planes[0].bytesused : buf.bytesused;
> > > +            if (bytesused == 0) {
> > > +                ctx->done = 1;
> > > +                return NULL;
> > > +            }
> > > +        }
> > > +
> > >          avbuf = &ctx->buffers[buf.index];
> > >          avbuf->status = V4L2BUF_AVAILABLE;
> > >          avbuf->buf = buf;
> > > --
> > > 2.25.1
> > > 
> 
> > 
> > Lgtm
> > 
> 
> Thanks. 
> Will apply both patches on Friday if no one objects.
> 

Applied both patches.

Thanks,
diff mbox series

Patch

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index 6c2db5c849..f0ecc18ebd 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -393,6 +393,15 @@  dequeue:
             return NULL;
         }
 
+        if (ctx_to_m2mctx(ctx)->draining && !V4L2_TYPE_IS_OUTPUT(ctx->type)) {
+            int bytesused = V4L2_TYPE_IS_MULTIPLANAR(buf.type) ?
+                            buf.m.planes[0].bytesused : buf.bytesused;
+            if (bytesused == 0) {
+                ctx->done = 1;
+                return NULL;
+            }
+        }
+
         avbuf = &ctx->buffers[buf.index];
         avbuf->status = V4L2BUF_AVAILABLE;
         avbuf->buf = buf;