diff mbox series

[FFmpeg-devel] avcodec/v4l2_m2m: handle the v4l2 eos event

Message ID 20200316020306.27607-1-ming.qian@nxp.com
State Superseded
Headers show
Series [FFmpeg-devel] avcodec/v4l2_m2m: handle the v4l2 eos event
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Ming Qian March 16, 2020, 2:03 a.m. UTC
when the last frame of capture is dequeueed,
driver may send this V4L2_EVENT_EOS event,
if this event is received, then we can set the capture port done

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_context.c |  5 +++++
 libavcodec/v4l2_m2m_dec.c | 10 ++++++++++
 libavcodec/v4l2_m2m_enc.c | 22 ++++++++++++++++++++++
 3 files changed, 37 insertions(+)

Comments

Andriy Gelman March 17, 2020, 4:20 a.m. UTC | #1
On Mon, 16. Mar 10:03, Ming Qian wrote:
> when the last frame of capture is dequeueed,
> driver may send this V4L2_EVENT_EOS event,
> if this event is received, then we can set the capture port done

Please add to your commit message (or something similar depending on what you
tested): 
"Without this patch the s5p-mfc often hangs at the end of encoding when flushing the
capture buffers." 

> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>  libavcodec/v4l2_context.c |  5 +++++
>  libavcodec/v4l2_m2m_dec.c | 10 ++++++++++
>  libavcodec/v4l2_m2m_enc.c | 22 ++++++++++++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index 8110bbb555..c10862aa12 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -171,6 +171,11 @@ static int v4l2_handle_event(V4L2Context *ctx)
>          return 0;
>      }
>  
> +    if (evt.type == V4L2_EVENT_EOS) {
> +        ctx->done = 1;
> +        return 0;
> +    }
> +
>      if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
>          return 0;
>  
> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> index d666edffe4..4862a4c0e5 100644
> --- a/libavcodec/v4l2_m2m_dec.c
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -123,6 +123,16 @@ static int v4l2_prepare_decoder(V4L2m2mContext *s)
>          }
>      }
>  

> +    memset(&sub, 0, sizeof(sub));
> +    sub.type = V4L2_EVENT_EOS;
> +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> +    if (ret < 0) {
> +        av_log(s->avctx, AV_LOG_ERROR,
> +                "the v4l2 driver does not support VIDIOC_SUBSCRIBE_EVENT\n"
> +                "you must provide an eos event to finish encode\n");
> +        return ret;
> +    }
> +
>      return 0;
>  }
>  
> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> index 5b954f4435..3abdd47a4a 100644
> --- a/libavcodec/v4l2_m2m_enc.c
> +++ b/libavcodec/v4l2_m2m_enc.c
> @@ -155,6 +155,24 @@ static int v4l2_check_b_frame_support(V4L2m2mContext *s)
>      return AVERROR_PATCHWELCOME;
>  }
>  
> +static int v4l2_subscribe_eos_event(V4L2m2mContext *s)
> +{
> +    struct v4l2_event_subscription sub;
> +    int ret;
> +
> +    memset(&sub, 0, sizeof(sub));
> +    sub.type = V4L2_EVENT_EOS;
> +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> +    if (ret < 0) {
> +        av_log(s->avctx, AV_LOG_ERROR,
> +                "the v4l2 driver does not support VIDIOC_SUBSCRIBE_EVENT\n"
> +                "you must provide an eos event to finish encode\n");
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
>  static int v4l2_prepare_encoder(V4L2m2mContext *s)
>  {
>      AVCodecContext *avctx = s->avctx;
> @@ -164,6 +182,10 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
>      /**
>       * requirements
>       */

> +    ret = v4l2_subscribe_eos_event(s);
> +    if (ret)
> +        return ret;
> +

I don't have a venus board to test, but it looks that the encoder doesn't support
V4L2_EVENT_EOS. It ends up calling v4l2_ctrl_subscribe_event() [1], which 
returns EINVAL. 

So I would log the error message, but not fail. Same in the decoder init. 
Ideally, it would be good to also test on a venus board too.

[1] https://github.com/torvalds/linux/blob/master/drivers/media/v4l2-core/v4l2-ctrls.c#L4565 

>      ret = v4l2_check_b_frame_support(s);
>      if (ret)
>          return ret;
> -- 
> 2.25.1
> 

Thanks,
BYHYKCHKIO WEIINZWLM March 17, 2020, 4:38 a.m. UTC | #2
You're welcome. Can this bug help me apply for a CVE?

On Tue, Mar 17, 2020 at 12:20 PM Andriy Gelman <andriy.gelman@gmail.com>
wrote:

> On Mon, 16. Mar 10:03, Ming Qian wrote:
> > when the last frame of capture is dequeueed,
> > driver may send this V4L2_EVENT_EOS event,
> > if this event is received, then we can set the capture port done
>
> Please add to your commit message (or something similar depending on what
> you
> tested):
> "Without this patch the s5p-mfc often hangs at the end of encoding when
> flushing the
> capture buffers."
>
> >
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> >  libavcodec/v4l2_context.c |  5 +++++
> >  libavcodec/v4l2_m2m_dec.c | 10 ++++++++++
> >  libavcodec/v4l2_m2m_enc.c | 22 ++++++++++++++++++++++
> >  3 files changed, 37 insertions(+)
> >
> > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> > index 8110bbb555..c10862aa12 100644
> > --- a/libavcodec/v4l2_context.c
> > +++ b/libavcodec/v4l2_context.c
> > @@ -171,6 +171,11 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >          return 0;
> >      }
> >
> > +    if (evt.type == V4L2_EVENT_EOS) {
> > +        ctx->done = 1;
> > +        return 0;
> > +    }
> > +
> >      if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
> >          return 0;
> >
> > diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> > index d666edffe4..4862a4c0e5 100644
> > --- a/libavcodec/v4l2_m2m_dec.c
> > +++ b/libavcodec/v4l2_m2m_dec.c
> > @@ -123,6 +123,16 @@ static int v4l2_prepare_decoder(V4L2m2mContext *s)
> >          }
> >      }
> >
>
> > +    memset(&sub, 0, sizeof(sub));
> > +    sub.type = V4L2_EVENT_EOS;
> > +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> > +    if (ret < 0) {
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +                "the v4l2 driver does not support
> VIDIOC_SUBSCRIBE_EVENT\n"
> > +                "you must provide an eos event to finish encode\n");
> > +        return ret;
> > +    }
> > +
> >      return 0;
> >  }
> >
> > diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> > index 5b954f4435..3abdd47a4a 100644
> > --- a/libavcodec/v4l2_m2m_enc.c
> > +++ b/libavcodec/v4l2_m2m_enc.c
> > @@ -155,6 +155,24 @@ static int
> v4l2_check_b_frame_support(V4L2m2mContext *s)
> >      return AVERROR_PATCHWELCOME;
> >  }
> >
> > +static int v4l2_subscribe_eos_event(V4L2m2mContext *s)
> > +{
> > +    struct v4l2_event_subscription sub;
> > +    int ret;
> > +
> > +    memset(&sub, 0, sizeof(sub));
> > +    sub.type = V4L2_EVENT_EOS;
> > +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> > +    if (ret < 0) {
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +                "the v4l2 driver does not support
> VIDIOC_SUBSCRIBE_EVENT\n"
> > +                "you must provide an eos event to finish encode\n");
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >  {
> >      AVCodecContext *avctx = s->avctx;
> > @@ -164,6 +182,10 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >      /**
> >       * requirements
> >       */
>
> > +    ret = v4l2_subscribe_eos_event(s);
> > +    if (ret)
> > +        return ret;
> > +
>
> I don't have a venus board to test, but it looks that the encoder doesn't
> support
> V4L2_EVENT_EOS. It ends up calling v4l2_ctrl_subscribe_event() [1], which
> returns EINVAL.
>
> So I would log the error message, but not fail. Same in the decoder init.
> Ideally, it would be good to also test on a venus board too.
>
> [1]
> https://github.com/torvalds/linux/blob/master/drivers/media/v4l2-core/v4l2-ctrls.c#L4565
>
> >      ret = v4l2_check_b_frame_support(s);
> >      if (ret)
> >          return ret;
> > --
> > 2.25.1
> >
>
> Thanks,
> --
> Andriy
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
BYHYKCHKIO WEIINZWLM March 17, 2020, 5:42 a.m. UTC | #3
I got a sample of the MP4 file through the fuzz program. When I load
this sample file through the program, the program crashes at the sws_scale
function.
    If you need sample files, please reply to me and I will send you sample
files.
[image: 图片2.png]
[image: 图片3.png]



On Tue, Mar 17, 2020 at 12:20 PM Andriy Gelman <andriy.gelman@gmail.com>
wrote:

> On Mon, 16. Mar 10:03, Ming Qian wrote:
> > when the last frame of capture is dequeueed,
> > driver may send this V4L2_EVENT_EOS event,
> > if this event is received, then we can set the capture port done
>
> Please add to your commit message (or something similar depending on what
> you
> tested):
> "Without this patch the s5p-mfc often hangs at the end of encoding when
> flushing the
> capture buffers."
>
> >
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> >  libavcodec/v4l2_context.c |  5 +++++
> >  libavcodec/v4l2_m2m_dec.c | 10 ++++++++++
> >  libavcodec/v4l2_m2m_enc.c | 22 ++++++++++++++++++++++
> >  3 files changed, 37 insertions(+)
> >
> > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> > index 8110bbb555..c10862aa12 100644
> > --- a/libavcodec/v4l2_context.c
> > +++ b/libavcodec/v4l2_context.c
> > @@ -171,6 +171,11 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >          return 0;
> >      }
> >
> > +    if (evt.type == V4L2_EVENT_EOS) {
> > +        ctx->done = 1;
> > +        return 0;
> > +    }
> > +
> >      if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
> >          return 0;
> >
> > diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> > index d666edffe4..4862a4c0e5 100644
> > --- a/libavcodec/v4l2_m2m_dec.c
> > +++ b/libavcodec/v4l2_m2m_dec.c
> > @@ -123,6 +123,16 @@ static int v4l2_prepare_decoder(V4L2m2mContext *s)
> >          }
> >      }
> >
>
> > +    memset(&sub, 0, sizeof(sub));
> > +    sub.type = V4L2_EVENT_EOS;
> > +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> > +    if (ret < 0) {
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +                "the v4l2 driver does not support
> VIDIOC_SUBSCRIBE_EVENT\n"
> > +                "you must provide an eos event to finish encode\n");
> > +        return ret;
> > +    }
> > +
> >      return 0;
> >  }
> >
> > diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> > index 5b954f4435..3abdd47a4a 100644
> > --- a/libavcodec/v4l2_m2m_enc.c
> > +++ b/libavcodec/v4l2_m2m_enc.c
> > @@ -155,6 +155,24 @@ static int
> v4l2_check_b_frame_support(V4L2m2mContext *s)
> >      return AVERROR_PATCHWELCOME;
> >  }
> >
> > +static int v4l2_subscribe_eos_event(V4L2m2mContext *s)
> > +{
> > +    struct v4l2_event_subscription sub;
> > +    int ret;
> > +
> > +    memset(&sub, 0, sizeof(sub));
> > +    sub.type = V4L2_EVENT_EOS;
> > +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> > +    if (ret < 0) {
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +                "the v4l2 driver does not support
> VIDIOC_SUBSCRIBE_EVENT\n"
> > +                "you must provide an eos event to finish encode\n");
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >  {
> >      AVCodecContext *avctx = s->avctx;
> > @@ -164,6 +182,10 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >      /**
> >       * requirements
> >       */
>
> > +    ret = v4l2_subscribe_eos_event(s);
> > +    if (ret)
> > +        return ret;
> > +
>
> I don't have a venus board to test, but it looks that the encoder doesn't
> support
> V4L2_EVENT_EOS. It ends up calling v4l2_ctrl_subscribe_event() [1], which
> returns EINVAL.
>
> So I would log the error message, but not fail. Same in the decoder init.
> Ideally, it would be good to also test on a venus board too.
>
> [1]
> https://github.com/torvalds/linux/blob/master/drivers/media/v4l2-core/v4l2-ctrls.c#L4565
>
> >      ret = v4l2_check_b_frame_support(s);
> >      if (ret)
> >          return ret;
> > --
> > 2.25.1
> >
>
> Thanks,
> --
> Andriy
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index 8110bbb555..c10862aa12 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -171,6 +171,11 @@  static int v4l2_handle_event(V4L2Context *ctx)
         return 0;
     }
 
+    if (evt.type == V4L2_EVENT_EOS) {
+        ctx->done = 1;
+        return 0;
+    }
+
     if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
         return 0;
 
diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
index d666edffe4..4862a4c0e5 100644
--- a/libavcodec/v4l2_m2m_dec.c
+++ b/libavcodec/v4l2_m2m_dec.c
@@ -123,6 +123,16 @@  static int v4l2_prepare_decoder(V4L2m2mContext *s)
         }
     }
 
+    memset(&sub, 0, sizeof(sub));
+    sub.type = V4L2_EVENT_EOS;
+    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
+    if (ret < 0) {
+        av_log(s->avctx, AV_LOG_ERROR,
+                "the v4l2 driver does not support VIDIOC_SUBSCRIBE_EVENT\n"
+                "you must provide an eos event to finish encode\n");
+        return ret;
+    }
+
     return 0;
 }
 
diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
index 5b954f4435..3abdd47a4a 100644
--- a/libavcodec/v4l2_m2m_enc.c
+++ b/libavcodec/v4l2_m2m_enc.c
@@ -155,6 +155,24 @@  static int v4l2_check_b_frame_support(V4L2m2mContext *s)
     return AVERROR_PATCHWELCOME;
 }
 
+static int v4l2_subscribe_eos_event(V4L2m2mContext *s)
+{
+    struct v4l2_event_subscription sub;
+    int ret;
+
+    memset(&sub, 0, sizeof(sub));
+    sub.type = V4L2_EVENT_EOS;
+    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
+    if (ret < 0) {
+        av_log(s->avctx, AV_LOG_ERROR,
+                "the v4l2 driver does not support VIDIOC_SUBSCRIBE_EVENT\n"
+                "you must provide an eos event to finish encode\n");
+        return ret;
+    }
+
+    return 0;
+}
+
 static int v4l2_prepare_encoder(V4L2m2mContext *s)
 {
     AVCodecContext *avctx = s->avctx;
@@ -164,6 +182,10 @@  static int v4l2_prepare_encoder(V4L2m2mContext *s)
     /**
      * requirements
      */
+    ret = v4l2_subscribe_eos_event(s);
+    if (ret)
+        return ret;
+
     ret = v4l2_check_b_frame_support(s);
     if (ret)
         return ret;