diff mbox

[FFmpeg-devel] lavf/vf_deinterlace_vaapi: flush queued frame in field mode

Message ID 1564739630-16675-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Fu, Linjie Aug. 2, 2019, 9:53 a.m. UTC
Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame
to flush the queued frame.

Fix the frame drop issue in field mode:

ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v
h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,
        deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'
-pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavfilter/vf_deinterlace_vaapi.c | 46 ++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 7 deletions(-)

Comments

Fu, Linjie Aug. 14, 2019, 2:01 a.m. UTC | #1
> -----Original Message-----
> From: Fu, Linjie
> Sent: Friday, August 2, 2019 17:54
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie <linjie.fu@intel.com>
> Subject: [PATCH] lavf/vf_deinterlace_vaapi: flush queued frame in field
> mode
> 
> Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame
> to flush the queued frame.
> 
> Fix the frame drop issue in field mode:
> 
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v
> h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,
>         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'
> -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavfilter/vf_deinterlace_vaapi.c | 46
> ++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/libavfilter/vf_deinterlace_vaapi.c
> b/libavfilter/vf_deinterlace_vaapi.c
> index 72d0349..c811327 100644
> --- a/libavfilter/vf_deinterlace_vaapi.c
> +++ b/libavfilter/vf_deinterlace_vaapi.c
> @@ -48,6 +48,9 @@ typedef struct DeintVAAPIContext {
>      int                queue_count;
>      AVFrame           *frame_queue[MAX_REFERENCES];
>      int                extra_delay_for_timestamps;
> +
> +    int                eof;
> +    int                prev_pts;
>  } DeintVAAPIContext;
> 
>  static const char *deint_vaapi_mode_name(int mode)
> @@ -190,11 +193,16 @@ static int deint_vaapi_filter_frame(AVFilterLink
> *inlink, AVFrame *input_frame)
>      void *filter_params_addr = NULL;
>      int err, i, field, current_frame_index;
> 
> -    av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n",
> -           av_get_pix_fmt_name(input_frame->format),
> -           input_frame->width, input_frame->height, input_frame->pts);
> +    // NULL frame is used to flush the queue in field mode
> +    if (input_frame)
> +        av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u
> (%"PRId64").\n",
> +            av_get_pix_fmt_name(input_frame->format),
> +            input_frame->width, input_frame->height, input_frame->pts);
> +    else if (ctx->field_rate == 1) {
> +        return 0;
> +    }
> 
> -    if (ctx->queue_count < ctx->queue_depth) {
> +    if (input_frame && ctx->queue_count < ctx->queue_depth) {
>          ctx->frame_queue[ctx->queue_count++] = input_frame;
>          if (ctx->queue_count < ctx->queue_depth) {
>              // Need more reference surfaces before we can continue.
> @@ -291,6 +299,8 @@ static int deint_vaapi_filter_frame(AVFilterLink *inlink,
> AVFrame *input_frame)
>          if (ctx->field_rate == 2) {
>              if (field == 0)
>                  output_frame->pts = 2 * input_frame->pts;
> +            else if (ctx->eof)
> +                output_frame->pts = 3 * input_frame->pts - ctx->prev_pts;
>              else
>                  output_frame->pts = input_frame->pts +
>                      ctx->frame_queue[current_frame_index + 1]->pts;
> @@ -306,6 +316,8 @@ static int deint_vaapi_filter_frame(AVFilterLink *inlink,
> AVFrame *input_frame)
>              break;
>      }
> 
> +    ctx->prev_pts = input_frame->pts;
> +
>      return err;
> 
>  fail:
> @@ -315,6 +327,25 @@ fail:
>      return err;
>  }
> 
> +static int deint_vaapi_request_frame(AVFilterLink *link)
> +{
> +    AVFilterContext *avctx = link->src;
> +    DeintVAAPIContext *ctx   = avctx->priv;
> +    int ret;
> +
> +    if (ctx->eof)
> +        return AVERROR_EOF;
> +
> +    ret = ff_request_frame(link->src->inputs[0]);
> +    if (ret == AVERROR_EOF) {
> +        ctx->eof = 1;
> +        deint_vaapi_filter_frame(link->src->inputs[0], NULL);
> +    } else if (ret < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
>  static av_cold int deint_vaapi_init(AVFilterContext *avctx)
>  {
>      VAAPIVPPContext *vpp_ctx = avctx->priv;
> @@ -376,9 +407,10 @@ static const AVFilterPad deint_vaapi_inputs[] = {
> 
>  static const AVFilterPad deint_vaapi_outputs[] = {
>      {
> -        .name = "default",
> -        .type = AVMEDIA_TYPE_VIDEO,
> -        .config_props = &deint_vaapi_config_output,
> +        .name           = "default",
> +        .type           = AVMEDIA_TYPE_VIDEO,
> +        .request_frame  = &deint_vaapi_request_frame,
> +        .config_props   = &deint_vaapi_config_output,
>      },
>      { NULL }
>  };
> --
> 2.7.4
Ping.
Aman Karmani Aug. 14, 2019, 2:04 a.m. UTC | #2
On Tue, Aug 13, 2019 at 7:01 PM Fu, Linjie <linjie.fu@intel.com> wrote:

> > -----Original Message-----
> > From: Fu, Linjie
> > Sent: Friday, August 2, 2019 17:54
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie <linjie.fu@intel.com>
> > Subject: [PATCH] lavf/vf_deinterlace_vaapi: flush queued frame in field
> > mode
> >
> > Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame
> > to flush the queued frame.
> >
> > Fix the frame drop issue in field mode:
> >
> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v
> > h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,
> >         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'
> > -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> >  libavfilter/vf_deinterlace_vaapi.c | 46
> > ++++++++++++++++++++++++++++++++------
> >  1 file changed, 39 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavfilter/vf_deinterlace_vaapi.c
> > b/libavfilter/vf_deinterlace_vaapi.c
> > index 72d0349..c811327 100644
> > --- a/libavfilter/vf_deinterlace_vaapi.c
> > +++ b/libavfilter/vf_deinterlace_vaapi.c
> > @@ -48,6 +48,9 @@ typedef struct DeintVAAPIContext {
> >      int                queue_count;
> >      AVFrame           *frame_queue[MAX_REFERENCES];
> >      int                extra_delay_for_timestamps;
> > +
> > +    int                eof;
> > +    int                prev_pts;
> >  } DeintVAAPIContext;
> >
> >  static const char *deint_vaapi_mode_name(int mode)
> > @@ -190,11 +193,16 @@ static int deint_vaapi_filter_frame(AVFilterLink
> > *inlink, AVFrame *input_frame)
> >      void *filter_params_addr = NULL;
> >      int err, i, field, current_frame_index;
> >
> > -    av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u
> (%"PRId64").\n",
> > -           av_get_pix_fmt_name(input_frame->format),
> > -           input_frame->width, input_frame->height, input_frame->pts);
> > +    // NULL frame is used to flush the queue in field mode
> > +    if (input_frame)
> > +        av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u
> > (%"PRId64").\n",
> > +            av_get_pix_fmt_name(input_frame->format),
> > +            input_frame->width, input_frame->height, input_frame->pts);
> > +    else if (ctx->field_rate == 1) {
> > +        return 0;
> > +    }
> >
> > -    if (ctx->queue_count < ctx->queue_depth) {
> > +    if (input_frame && ctx->queue_count < ctx->queue_depth) {
> >          ctx->frame_queue[ctx->queue_count++] = input_frame;
> >          if (ctx->queue_count < ctx->queue_depth) {
> >              // Need more reference surfaces before we can continue.
> > @@ -291,6 +299,8 @@ static int deint_vaapi_filter_frame(AVFilterLink
> *inlink,
> > AVFrame *input_frame)
> >          if (ctx->field_rate == 2) {
> >              if (field == 0)
> >                  output_frame->pts = 2 * input_frame->pts;
> > +            else if (ctx->eof)
> > +                output_frame->pts = 3 * input_frame->pts -
> ctx->prev_pts;
> >              else
> >                  output_frame->pts = input_frame->pts +
> >                      ctx->frame_queue[current_frame_index + 1]->pts;
> > @@ -306,6 +316,8 @@ static int deint_vaapi_filter_frame(AVFilterLink
> *inlink,
> > AVFrame *input_frame)
> >              break;
> >      }
> >
> > +    ctx->prev_pts = input_frame->pts;
> > +
> >      return err;
> >
> >  fail:
> > @@ -315,6 +327,25 @@ fail:
> >      return err;
> >  }
> >
> > +static int deint_vaapi_request_frame(AVFilterLink *link)
> > +{
> > +    AVFilterContext *avctx = link->src;
> > +    DeintVAAPIContext *ctx   = avctx->priv;
> > +    int ret;
> > +
> > +    if (ctx->eof)
> > +        return AVERROR_EOF;
> > +
> > +    ret = ff_request_frame(link->src->inputs[0]);
> > +    if (ret == AVERROR_EOF) {
> > +        ctx->eof = 1;
> > +        deint_vaapi_filter_frame(link->src->inputs[0], NULL);
> > +    } else if (ret < 0)
> > +        return ret;
> > +
> > +    return 0;
> > +}
> > +
> >  static av_cold int deint_vaapi_init(AVFilterContext *avctx)
> >  {
> >      VAAPIVPPContext *vpp_ctx = avctx->priv;
> > @@ -376,9 +407,10 @@ static const AVFilterPad deint_vaapi_inputs[] = {
> >
> >  static const AVFilterPad deint_vaapi_outputs[] = {
> >      {
> > -        .name = "default",
> > -        .type = AVMEDIA_TYPE_VIDEO,
> > -        .config_props = &deint_vaapi_config_output,
> > +        .name           = "default",
> > +        .type           = AVMEDIA_TYPE_VIDEO,
> > +        .request_frame  = &deint_vaapi_request_frame,
> > +        .config_props   = &deint_vaapi_config_output,
> >      },
> >      { NULL }
> >  };
> > --
> > 2.7.4
> Ping.
>

LGTM


> _______________________________________________
> 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".
Fu, Linjie Aug. 19, 2019, 1:22 a.m. UTC | #3
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Aman Gupta

> Sent: Wednesday, August 14, 2019 10:05

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH] lavf/vf_deinterlace_vaapi: flush

> queued frame in field mode

> 

> On Tue, Aug 13, 2019 at 7:01 PM Fu, Linjie <linjie.fu@intel.com> wrote:

> 

> > > -----Original Message-----

> > > From: Fu, Linjie

> > > Sent: Friday, August 2, 2019 17:54

> > > To: ffmpeg-devel@ffmpeg.org

> > > Cc: Fu, Linjie <linjie.fu@intel.com>

> > > Subject: [PATCH] lavf/vf_deinterlace_vaapi: flush queued frame in field

> > > mode

> > >

> > > Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame

> > > to flush the queued frame.

> > >

> > > Fix the frame drop issue in field mode:

> > >

> > > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -

> c:v

> > > h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,

> > >         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'

> > > -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv

> > >

> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > > ---

> > >  libavfilter/vf_deinterlace_vaapi.c | 46

> > > ++++++++++++++++++++++++++++++++------

> > >  1 file changed, 39 insertions(+), 7 deletions(-)

> > >

> > > diff --git a/libavfilter/vf_deinterlace_vaapi.c

> > > b/libavfilter/vf_deinterlace_vaapi.c

> > > index 72d0349..c811327 100644

> > > --- a/libavfilter/vf_deinterlace_vaapi.c

> > > +++ b/libavfilter/vf_deinterlace_vaapi.c

> > > @@ -48,6 +48,9 @@ typedef struct DeintVAAPIContext {

> > >      int                queue_count;

> > >      AVFrame           *frame_queue[MAX_REFERENCES];

> > >      int                extra_delay_for_timestamps;

> > > +

> > > +    int                eof;

> > > +    int                prev_pts;

> > >  } DeintVAAPIContext;

> > >

> > >  static const char *deint_vaapi_mode_name(int mode)

> > > @@ -190,11 +193,16 @@ static int deint_vaapi_filter_frame(AVFilterLink

> > > *inlink, AVFrame *input_frame)

> > >      void *filter_params_addr = NULL;

> > >      int err, i, field, current_frame_index;

> > >

> > > -    av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u

> > (%"PRId64").\n",

> > > -           av_get_pix_fmt_name(input_frame->format),

> > > -           input_frame->width, input_frame->height, input_frame->pts);

> > > +    // NULL frame is used to flush the queue in field mode

> > > +    if (input_frame)

> > > +        av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u

> > > (%"PRId64").\n",

> > > +            av_get_pix_fmt_name(input_frame->format),

> > > +            input_frame->width, input_frame->height, input_frame->pts);

> > > +    else if (ctx->field_rate == 1) {

> > > +        return 0;

> > > +    }

> > >

> > > -    if (ctx->queue_count < ctx->queue_depth) {

> > > +    if (input_frame && ctx->queue_count < ctx->queue_depth) {

> > >          ctx->frame_queue[ctx->queue_count++] = input_frame;

> > >          if (ctx->queue_count < ctx->queue_depth) {

> > >              // Need more reference surfaces before we can continue.

> > > @@ -291,6 +299,8 @@ static int deint_vaapi_filter_frame(AVFilterLink

> > *inlink,

> > > AVFrame *input_frame)

> > >          if (ctx->field_rate == 2) {

> > >              if (field == 0)

> > >                  output_frame->pts = 2 * input_frame->pts;

> > > +            else if (ctx->eof)

> > > +                output_frame->pts = 3 * input_frame->pts -

> > ctx->prev_pts;

> > >              else

> > >                  output_frame->pts = input_frame->pts +

> > >                      ctx->frame_queue[current_frame_index + 1]->pts;

> > > @@ -306,6 +316,8 @@ static int deint_vaapi_filter_frame(AVFilterLink

> > *inlink,

> > > AVFrame *input_frame)

> > >              break;

> > >      }

> > >

> > > +    ctx->prev_pts = input_frame->pts;

> > > +

> > >      return err;

> > >

> > >  fail:

> > > @@ -315,6 +327,25 @@ fail:

> > >      return err;

> > >  }

> > >

> > > +static int deint_vaapi_request_frame(AVFilterLink *link)

> > > +{

> > > +    AVFilterContext *avctx = link->src;

> > > +    DeintVAAPIContext *ctx   = avctx->priv;

> > > +    int ret;

> > > +

> > > +    if (ctx->eof)

> > > +        return AVERROR_EOF;

> > > +

> > > +    ret = ff_request_frame(link->src->inputs[0]);

> > > +    if (ret == AVERROR_EOF) {

> > > +        ctx->eof = 1;

> > > +        deint_vaapi_filter_frame(link->src->inputs[0], NULL);

> > > +    } else if (ret < 0)

> > > +        return ret;

> > > +

> > > +    return 0;

> > > +}

> > > +

> > >  static av_cold int deint_vaapi_init(AVFilterContext *avctx)

> > >  {

> > >      VAAPIVPPContext *vpp_ctx = avctx->priv;

> > > @@ -376,9 +407,10 @@ static const AVFilterPad deint_vaapi_inputs[] = {

> > >

> > >  static const AVFilterPad deint_vaapi_outputs[] = {

> > >      {

> > > -        .name = "default",

> > > -        .type = AVMEDIA_TYPE_VIDEO,

> > > -        .config_props = &deint_vaapi_config_output,

> > > +        .name           = "default",

> > > +        .type           = AVMEDIA_TYPE_VIDEO,

> > > +        .request_frame  = &deint_vaapi_request_frame,

> > > +        .config_props   = &deint_vaapi_config_output,

> > >      },

> > >      { NULL }

> > >  };

> > > --

> > > 2.7.4

> > Ping.

> >

> 

> LGTM

> 

Thanks for the review.
ping for merge, or other comments?

- linjie
Mark Thompson Aug. 20, 2019, 8:49 p.m. UTC | #4
On 02/08/2019 10:53, Linjie Fu wrote:
> Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame
> to flush the queued frame.
> 
> Fix the frame drop issue in field mode:
> 
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v
> h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,
>         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'
> -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavfilter/vf_deinterlace_vaapi.c | 46 ++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 7 deletions(-)

Can you explain in more detail what the problem is here?  What frame is being dropped?


I already get the expected number of frames out with both rate settings (input total - forward references - backward references, all multiplied by 2 for field rate output).  E.g. for 100 frames/field-pairs of input:

$ ./ffmpeg_g -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -hwaccel_output_format vaapi -i f100.mp4 -an -vf deinterlace_vaapi=rate=frame -f null -
...
frame=   97

$ ./ffmpeg_g -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -hwaccel_output_format vaapi -i f100.mp4 -an -vf deinterlace_vaapi=rate=field -f null -
...
frame=  194

(With forward = 2, backward = 1.)


With this patch applied, the filter always segfaults for me at the end of the stream when set to field rate:

$ gdb --args ./ffmpeg_g -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -hwaccel_output_format vaapi -i f100.mp4 -an -vf deinterlace_vaapi=rate=field -f null -
...
Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.
0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300, input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227
227                 ctx->frame_queue[current_frame_index + i + 1]->data[3];
(gdb) bt
#0  0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300, input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227
#1  0x00005555557565d2 in deint_vaapi_request_frame (link=0x5555590928c0) at src/libavfilter/vf_deinterlace_vaapi.c:342
#2  0x00005555556ce950 in ff_request_frame_to_filter (link=0x5555590928c0) at src/libavfilter/avfilter.c:458
#3  0x00005555556d0cd0 in forward_status_change (filter=0x555559088940, in=0x555559092300) at src/libavfilter/avfilter.c:1243
#4  0x00005555556d0eaf in ff_filter_activate_default (filter=0x555559088940) at src/libavfilter/avfilter.c:1274
#5  0x00005555556d0fec in ff_filter_activate (filter=0x555559088940) at src/libavfilter/avfilter.c:1430
#6  0x00005555556d5d29 in ff_filter_graph_run_once (graph=0x55555908ff00) at src/libavfilter/avfiltergraph.c:1456
#7  0x00005555556d71d8 in push_frame (graph=0x55555908ff00) at src/libavfilter/buffersrc.c:187
#8  0x00005555556d77a9 in av_buffersrc_close (ctx=0x555559091ac0, pts=300300, flags=4) at src/libavfilter/buffersrc.c:275
#9  0x000055555569356a in ifilter_send_eof (ifilter=0x5555580d0e40, pts=300300) at src/fftools/ffmpeg.c:2213
#10 0x00005555556949fd in send_filter_eof (ist=0x5555580f3ac0) at src/fftools/ffmpeg.c:2562
#11 0x0000555555695336 in process_input_packet (ist=0x5555580f3ac0, pkt=0x0, no_eof=0) at src/fftools/ffmpeg.c:2701
#12 0x000055555569a9db in process_input (file_index=0) at src/fftools/ffmpeg.c:4313
#13 0x000055555569c5f9 in transcode_step () at src/fftools/ffmpeg.c:4638
#14 0x000055555569c726 in transcode () at src/fftools/ffmpeg.c:4692
#15 0x000055555569cfb6 in main (argc=15, argv=0x7fffffffe488) at src/fftools/ffmpeg.c:4894
(gdb) p current_frame_index 
$1 = 2
(gdb) p i
$2 = 0
(gdb) p ctx->frame_queue[current_frame_index + i + 1]
$3 = (AVFrame *) 0x0


- Mark
Fu, Linjie Aug. 21, 2019, 12:21 a.m. UTC | #5
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Mark Thompson

> Sent: Wednesday, August 21, 2019 04:49

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] lavf/vf_deinterlace_vaapi: flush

> queued frame in field mode

> 

> On 02/08/2019 10:53, Linjie Fu wrote:

> > Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame

> > to flush the queued frame.

> >

> > Fix the frame drop issue in field mode:

> >

> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v

> > h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,

> >         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'

> > -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> >  libavfilter/vf_deinterlace_vaapi.c | 46

> ++++++++++++++++++++++++++++++++------

> >  1 file changed, 39 insertions(+), 7 deletions(-)

> 

> Can you explain in more detail what the problem is here?  What frame is

> being dropped?



Would you please try this clips to see whether it could be reproduced on your side?
https://drive.google.com/open?id=11FZlT0vl_GoGjlEuihpZHgbwgVA7yGUV


$ ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -hwaccel_output_format vaapi -v verbose -c:v h264 -i ./input.h264 -vf 'hwupload,deinterlace_vaapi=mode=bob:rate=frame' -f null -

2 frames

$ ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -hwaccel_output_format vaapi -v verbose -c:v h264 -i ./input.h264 -vf 'hwupload,deinterlace_vaapi=mode=bob:rate=field' -f null -

Before applying:
2 frames

After applying:
4 frames 

> With this patch applied, the filter always segfaults for me at the end of the

> stream when set to field rate:


This patch just sends a NULL frame to deint_vaapi_request_frame() at the end.
Since the input_frame is not NULL in the log, it's kind of weird if it triggers a segfault.

Is it possible to share the clips to reproduce this?

> $ gdb --args ./ffmpeg_g -hwaccel vaapi -hwaccel_device

> /dev/dri/renderD129 -hwaccel_output_format vaapi -i f100.mp4 -an -vf

> deinterlace_vaapi=rate=field -f null -

> ...

> Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.

> 0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300,

> input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227

> 227                 ctx->frame_queue[current_frame_index + i + 1]->data[3];

> (gdb) bt

> #0  0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300,

> input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227



Thanks for the response.

- linjie
Fu, Linjie Sept. 18, 2019, 8:09 a.m. UTC | #6
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Mark Thompson

> Sent: Wednesday, August 21, 2019 04:49

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] lavf/vf_deinterlace_vaapi: flush

> queued frame in field mode

> 

> On 02/08/2019 10:53, Linjie Fu wrote:

> > Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame

> > to flush the queued frame.

> >

> > Fix the frame drop issue in field mode:

> >

> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -c:v

> > h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,

> >         deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'

> > -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> >  libavfilter/vf_deinterlace_vaapi.c | 46

> ++++++++++++++++++++++++++++++++------

> >  1 file changed, 39 insertions(+), 7 deletions(-)

> 

> Can you explain in more detail what the problem is here?  What frame is

> being dropped?


For DeinterlacingBob mode with rate=field, the frame number of output
should equal 2x input total since only intra deinterlace is used.

Currently for "pipeline_caps.num_backward_references, rate = field",
extra_delay is introduced. Due to the async without flush, frame number
of output is [expected_number - 2].

Specifically, if the input only has 1 frame, the output will be empty.

For 1 frame input in Bob mode with rate=field,
before patch: 0 frame;
after  patch: 2 frames;

> 

> I already get the expected number of frames out with both rate settings

> (input total - forward references - backward references, all multiplied by 2

> for field rate output).  E.g. for 100 frames/field-pairs of input:

> 

> $ ./ffmpeg_g -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -

> hwaccel_output_format vaapi -i f100.mp4 -an -vf

> deinterlace_vaapi=rate=frame -f null -

> ...

> frame=   97

> 

> $ ./ffmpeg_g -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -

> hwaccel_output_format vaapi -i f100.mp4 -an -vf

> deinterlace_vaapi=rate=field -f null -

> ...

> frame=  194

> 

> (With forward = 2, backward = 1.)


Which driver is used for this test?

I've verified with i965 and iHD on linux and failed to find the matched
pipeline_cap with num_forward_reference = 2, num_backward_reference = 1;

For iHD:

#define DDI_CODEC_NUM_FWD_REF 0;
#define DDI_CODEC_NUM_BK_REF 0;

num_forward_reference = DDI_CODEC_NUM_FWD_REF; 
num_backward_reference = DDI_CODEC_NUM_BK_REF;

https://github.com/intel/media-driver/blob/b9606584bf3dc6937047b4f19e08c75f88bd95a4/media_driver/linux/common/ddi/media_libva.cpp#L5687

For i965:
num_forward_reference ++; (if working on MotionAdaptive or MotionCompensated)
num_backward_reference = 0;

https://github.com/intel/intel-vaapi-driver/blob/9bc30a0231e55f17afed50589669d11e844d0bb9/src/i965_drv_video.c#L7158

> 

> With this patch applied, the filter always segfaults for me at the end of the

> stream when set to field rate:

> 

> $ gdb --args ./ffmpeg_g -hwaccel vaapi -hwaccel_device

> /dev/dri/renderD129 -hwaccel_output_format vaapi -i f100.mp4 -an -vf

> deinterlace_vaapi=rate=field -f null -

> ...

> Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.

> 0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300,

> input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227

> 227                 ctx->frame_queue[current_frame_index + i + 1]->data[3];

> (gdb) bt

> #0  0x0000555555755fb1 in deint_vaapi_filter_frame (inlink=0x555559092300,

> input_frame=0x555558d42000) at src/libavfilter/vf_deinterlace_vaapi.c:227

> #1  0x00005555557565d2 in deint_vaapi_request_frame

> (link=0x5555590928c0) at src/libavfilter/vf_deinterlace_vaapi.c:342

> #2  0x00005555556ce950 in ff_request_frame_to_filter (link=0x5555590928c0)

> at src/libavfilter/avfilter.c:458

> #3  0x00005555556d0cd0 in forward_status_change (filter=0x555559088940,

> in=0x555559092300) at src/libavfilter/avfilter.c:1243

> #4  0x00005555556d0eaf in ff_filter_activate_default (filter=0x555559088940)

> at src/libavfilter/avfilter.c:1274

> #5  0x00005555556d0fec in ff_filter_activate (filter=0x555559088940) at

> src/libavfilter/avfilter.c:1430

> #6  0x00005555556d5d29 in ff_filter_graph_run_once (graph=0x55555908ff00)

> at src/libavfilter/avfiltergraph.c:1456

> #7  0x00005555556d71d8 in push_frame (graph=0x55555908ff00) at

> src/libavfilter/buffersrc.c:187

> #8  0x00005555556d77a9 in av_buffersrc_close (ctx=0x555559091ac0,

> pts=300300, flags=4) at src/libavfilter/buffersrc.c:275

> #9  0x000055555569356a in ifilter_send_eof (ifilter=0x5555580d0e40,

> pts=300300) at src/fftools/ffmpeg.c:2213

> #10 0x00005555556949fd in send_filter_eof (ist=0x5555580f3ac0) at

> src/fftools/ffmpeg.c:2562

> #11 0x0000555555695336 in process_input_packet (ist=0x5555580f3ac0,

> pkt=0x0, no_eof=0) at src/fftools/ffmpeg.c:2701

> #12 0x000055555569a9db in process_input (file_index=0) at

> src/fftools/ffmpeg.c:4313

> #13 0x000055555569c5f9 in transcode_step () at src/fftools/ffmpeg.c:4638

> #14 0x000055555569c726 in transcode () at src/fftools/ffmpeg.c:4692

> #15 0x000055555569cfb6 in main (argc=15, argv=0x7fffffffe488) at

> src/fftools/ffmpeg.c:4894

> (gdb) p current_frame_index

> $1 = 2

> (gdb) p i

> $2 = 0

> (gdb) p ctx->frame_queue[current_frame_index + i + 1]

> $3 = (AVFrame *) 0x0

> 


The last frame required a backward_reference which is not existed.
It seems that the flush is redundant under this condition
(pipeline_caps.num_backward_reference  > 0).

A possible solution is to flush the queue only when extra_delay is needed
for timestamps.(Not verified yet since couldn't got the same pipeline_caps 
environment )

New patch will be sent later.

- linjie
diff mbox

Patch

diff --git a/libavfilter/vf_deinterlace_vaapi.c b/libavfilter/vf_deinterlace_vaapi.c
index 72d0349..c811327 100644
--- a/libavfilter/vf_deinterlace_vaapi.c
+++ b/libavfilter/vf_deinterlace_vaapi.c
@@ -48,6 +48,9 @@  typedef struct DeintVAAPIContext {
     int                queue_count;
     AVFrame           *frame_queue[MAX_REFERENCES];
     int                extra_delay_for_timestamps;
+
+    int                eof;
+    int                prev_pts;
 } DeintVAAPIContext;
 
 static const char *deint_vaapi_mode_name(int mode)
@@ -190,11 +193,16 @@  static int deint_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
     void *filter_params_addr = NULL;
     int err, i, field, current_frame_index;
 
-    av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n",
-           av_get_pix_fmt_name(input_frame->format),
-           input_frame->width, input_frame->height, input_frame->pts);
+    // NULL frame is used to flush the queue in field mode
+    if (input_frame)
+        av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n",
+            av_get_pix_fmt_name(input_frame->format),
+            input_frame->width, input_frame->height, input_frame->pts);
+    else if (ctx->field_rate == 1) {
+        return 0;
+    }
 
-    if (ctx->queue_count < ctx->queue_depth) {
+    if (input_frame && ctx->queue_count < ctx->queue_depth) {
         ctx->frame_queue[ctx->queue_count++] = input_frame;
         if (ctx->queue_count < ctx->queue_depth) {
             // Need more reference surfaces before we can continue.
@@ -291,6 +299,8 @@  static int deint_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
         if (ctx->field_rate == 2) {
             if (field == 0)
                 output_frame->pts = 2 * input_frame->pts;
+            else if (ctx->eof)
+                output_frame->pts = 3 * input_frame->pts - ctx->prev_pts;
             else
                 output_frame->pts = input_frame->pts +
                     ctx->frame_queue[current_frame_index + 1]->pts;
@@ -306,6 +316,8 @@  static int deint_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
             break;
     }
 
+    ctx->prev_pts = input_frame->pts;
+
     return err;
 
 fail:
@@ -315,6 +327,25 @@  fail:
     return err;
 }
 
+static int deint_vaapi_request_frame(AVFilterLink *link)
+{
+    AVFilterContext *avctx = link->src;
+    DeintVAAPIContext *ctx   = avctx->priv;
+    int ret;
+
+    if (ctx->eof)
+        return AVERROR_EOF;
+
+    ret = ff_request_frame(link->src->inputs[0]);
+    if (ret == AVERROR_EOF) {
+        ctx->eof = 1;
+        deint_vaapi_filter_frame(link->src->inputs[0], NULL);
+    } else if (ret < 0)
+        return ret;
+
+    return 0;
+}
+
 static av_cold int deint_vaapi_init(AVFilterContext *avctx)
 {
     VAAPIVPPContext *vpp_ctx = avctx->priv;
@@ -376,9 +407,10 @@  static const AVFilterPad deint_vaapi_inputs[] = {
 
 static const AVFilterPad deint_vaapi_outputs[] = {
     {
-        .name = "default",
-        .type = AVMEDIA_TYPE_VIDEO,
-        .config_props = &deint_vaapi_config_output,
+        .name           = "default",
+        .type           = AVMEDIA_TYPE_VIDEO,
+        .request_frame  = &deint_vaapi_request_frame,
+        .config_props   = &deint_vaapi_config_output,
     },
     { NULL }
 };