diff mbox series

[FFmpeg-devel] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input

Message ID 1582903831-20063-1-git-send-email-linjie.fu@intel.com
State New
Headers show
Series [FFmpeg-devel] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input
Related show

Checks

Context Check Description
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

Fu, Linjie Feb. 28, 2020, 3:30 p.m. UTC
There are frame droppings in frc while converting into a lower framerate,
and MSDK returns ERROR_MORE_DATA which should be ignored.

Reported-by: Gupta, Pallavi <pallavi.gupta@intel.com>
Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
Could be reproduced by:
ffmpeg -hwaccel qsv -c:v hevc_qsv -i input_25fps.h265
        -vf vpp_qsv=framerate=20
        -c:v hevc_qsv -b:v 4M 4M20FPS.mp4

 libavfilter/vf_vpp_qsv.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Zhong Li Feb. 29, 2020, 5:13 a.m. UTC | #1
Linjie Fu <linjie.fu@intel.com> 于2020年2月28日周五 下午11:34写道:
>
> There are frame droppings in frc while converting into a lower framerate,
> and MSDK returns ERROR_MORE_DATA which should be ignored.

Should be fixed in MSDK instead of working around in FFmpeg?
Fu, Linjie Feb. 29, 2020, 9:35 a.m. UTC | #2
> -----Original Message-----
> From: Zhong Li <lizhong1008@gmail.com>
> Sent: Saturday, February 29, 2020 13:14
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop
> while framerate lower than input
> 
> Linjie Fu <linjie.fu@intel.com> 于2020年2月28日周五 下午11:34写道:
> >
> > There are frame droppings in frc while converting into a lower framerate,
> > and MSDK returns ERROR_MORE_DATA which should be ignored.
> 
> Should be fixed in MSDK instead of working around in FFmpeg?

MSDK made decision regarding frame rate conversion. If it's the framerate down case, 
FRC would skip frame without producing an output [1], and request a new input frame.

This seems to match the description in mediasdk-man.md [2]:
MFX_ERR_MORE_DATA: Need more input frames before VPP can produce an output

[1] https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/mfx_lib/vpp/src/mfx_vpp_hw.cpp#L324
[2] https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/mediasdk-man.md#mfxvideovpp_runframevppasync
Zhong Li April 19, 2020, 3 p.m. UTC | #3
Fu, Linjie <linjie.fu@intel.com> 于2020年2月29日周六 下午5:35写道:
>
> > -----Original Message-----
> > From: Zhong Li <lizhong1008@gmail.com>
> > Sent: Saturday, February 29, 2020 13:14
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Cc: Fu, Linjie <linjie.fu@intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop
> > while framerate lower than input
> >
> > Linjie Fu <linjie.fu@intel.com> 于2020年2月28日周五 下午11:34写道:
> > >
> > > There are frame droppings in frc while converting into a lower framerate,
> > > and MSDK returns ERROR_MORE_DATA which should be ignored.
> >
> > Should be fixed in MSDK instead of working around in FFmpeg?
>
> MSDK made decision regarding frame rate conversion. If it's the framerate down case,
> FRC would skip frame without producing an output [1], and request a new input frame.

I can't see the benefit to use MSDK framerate conversion function. Is
it a good idea to drop it and use ffmpeg native fps filter instead?
Fu, Linjie April 21, 2020, 5:21 a.m. UTC | #4
> From: Zhong Li <lizhong1008@gmail.com>
> Sent: Sunday, April 19, 2020 23:00
> To: Fu, Linjie <linjie.fu@intel.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop
> while framerate lower than input
> 
> Fu, Linjie <linjie.fu@intel.com> 于2020年2月29日周六 下午5:35写道:
> >
> > > -----Original Message-----
> > > From: Zhong Li <lizhong1008@gmail.com>
> > > Sent: Saturday, February 29, 2020 13:14
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Cc: Fu, Linjie <linjie.fu@intel.com>
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite
> loop
> > > while framerate lower than input
> > >
> > > Linjie Fu <linjie.fu@intel.com> 于2020年2月28日周五 下午11:34写
> 道:
> > > >
> > > > There are frame droppings in frc while converting into a lower
> framerate,
> > > > and MSDK returns ERROR_MORE_DATA which should be ignored.
> > >
> > > Should be fixed in MSDK instead of working around in FFmpeg?
> >
> > MSDK made decision regarding frame rate conversion. If it's the framerate
> down case,
> > FRC would skip frame without producing an output [1], and request a new
> input frame.
> 
> I can't see the benefit to use MSDK framerate conversion function. Is
> it a good idea to drop it and use ffmpeg native fps filter instead?

The implementation of FRC inside MSDK is quite straight-forward or simple
currently since it just drops or repeats frames, hence I think using native fps
filter is a good idea for decoding + FRC or FRC + encoding.

However, for a pure hardware transcoding pipeline, there may be some
performance issues if inserting a software filter, extra memory copy would
be introduced in hwdownload/hwupload between system memory and video
memory, which would impact a lot for large resolutions.

Took a simple tests for 4K 10bit HEVC transcoding:

# inserting a fps filter in the transcoding: 20 fps
$ ffmpeg -hwaccel qsv -c:v hevc_qsv -i ../Exodus_UHD_HDR_Exodus_draft.mp4 -vf "hwdownload,format=p010le,fps=fps=60,hwupload=extra_hw_frames=40" -c:v hevc_qsv out.mp4

# using msdk framerate conversion: 33 fps
$ ffmpeg -hwaccel qsv -c:v hevc_qsv -i ../Exodus_UHD_HDR_Exodus_draft.mp4 -vf "vpp_qsv=framerate=60" -c:v hevc_qsv out.mp4

- Linjie
Zhong Li April 21, 2020, 9:09 a.m. UTC | #5
> > I can't see the benefit to use MSDK framerate conversion function. Is
> > it a good idea to drop it and use ffmpeg native fps filter instead?
>
> The implementation of FRC inside MSDK is quite straight-forward or simple
> currently since it just drops or repeats frames, hence I think using native fps
> filter is a good idea for decoding + FRC or FRC + encoding.
>
> However, for a pure hardware transcoding pipeline, there may be some
> performance issues if inserting a software filter, extra memory copy would
> be introduced in hwdownload/hwupload between system memory and video
> memory, which would impact a lot for large resolutions.
IIRC, it is not necessary to insert hwdownload/upload to use fps filter. No ?
Fu, Linjie April 21, 2020, 9:27 a.m. UTC | #6
> From: Zhong Li <lizhong1008@gmail.com>
> Sent: Tuesday, April 21, 2020 17:10
> To: Fu, Linjie <linjie.fu@intel.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop
> while framerate lower than input
> 
> > > I can't see the benefit to use MSDK framerate conversion function. Is
> > > it a good idea to drop it and use ffmpeg native fps filter instead?
> >
> > The implementation of FRC inside MSDK is quite straight-forward or simple
> > currently since it just drops or repeats frames, hence I think using native
> fps
> > filter is a good idea for decoding + FRC or FRC + encoding.
> >
> > However, for a pure hardware transcoding pipeline, there may be some
> > performance issues if inserting a software filter, extra memory copy would
> > be introduced in hwdownload/hwupload between system memory and
> video
> > memory, which would impact a lot for large resolutions.
> IIRC, it is not necessary to insert hwdownload/upload to use fps filter. No ?
Ahh, you are right, the native fps filter would be fair enough for this case.

Thanks for the elaborations.
Fu, Linjie June 30, 2020, 6:29 a.m. UTC | #7
> From: Zhong Li <lizhong1008@gmail.com>
> Sent: Sunday, April 19, 2020 23:00
> To: Fu, Linjie <linjie.fu@intel.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop
> while framerate lower than input
> 
> Fu, Linjie <linjie.fu@intel.com> 于2020年2月29日周六 下午5:35写道:
> >
> > > -----Original Message-----
> > > From: Zhong Li <lizhong1008@gmail.com>
> > > Sent: Saturday, February 29, 2020 13:14
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Cc: Fu, Linjie <linjie.fu@intel.com>
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite
> loop
> > > while framerate lower than input
> > >
> > > Linjie Fu <linjie.fu@intel.com> 于2020年2月28日周五 下午11:34写
> 道:
> > > >
> > > > There are frame droppings in frc while converting into a lower
> framerate,
> > > > and MSDK returns ERROR_MORE_DATA which should be ignored.
> > >
> > > Should be fixed in MSDK instead of working around in FFmpeg?
> >
> > MSDK made decision regarding frame rate conversion. If it's the framerate
> down case,
> > FRC would skip frame without producing an output [1], and request a new
> input frame.
> 
> I can't see the benefit to use MSDK framerate conversion function. Is
> it a good idea to drop it and use ffmpeg native fps filter instead?

Reconsidering this, leaving the filter buggy doesn't seem to be comfortable to me,
hence IMHO it'll be better to have this bug fixed.

Ping for this, thx.

- Linjie
Zhong Li July 5, 2020, 3:49 p.m. UTC | #8
> > I can't see the benefit to use MSDK framerate conversion function. Is
> > it a good idea to drop it and use ffmpeg native fps filter instead?
>
> Reconsidering this, leaving the filter buggy doesn't seem to be comfortable to me,
> hence IMHO it'll be better to have this bug fixed.
My suggestion would be just delete MSDK framerate conversion instead
of patch it.
Will send a patch if nobody against.
Max Dmitrichenko July 5, 2020, 9:32 p.m. UTC | #9
On Sun, Jul 5, 2020 at 5:56 PM Zhong Li <lizhong1008@gmail.com> wrote:

> > > I can't see the benefit to use MSDK framerate conversion function. Is
> > > it a good idea to drop it and use ffmpeg native fps filter instead?
> >
> > Reconsidering this, leaving the filter buggy doesn't seem to be
> comfortable to me,
> > hence IMHO it'll be better to have this bug fixed.
> My suggestion would be just delete MSDK framerate conversion instead
> of patch it.
> Will send a patch if nobody against.
>

should be ok when comes with proper description,
as comes with price of performance

regards
Max
diff mbox series

Patch

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 8585874..0218c2a 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -476,6 +476,9 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
 
     if (vpp->qsv) {
         ret = ff_qsvvpp_filter_frame(vpp->qsv, inlink, picref);
+        /* ignore the EAGAIN caused by frame dropping in frc */
+        if (ret == AVERROR(EAGAIN))
+            ret = av_cmp_q(vpp->framerate, inlink->frame_rate) < 0 ? 0 : ret;
         av_frame_free(&picref);
     } else {
         if (picref->pts != AV_NOPTS_VALUE)