On Fri, Apr 28, 2017 at 11:23:10PM +0700, Muhammad Faiz wrote: > On Fri, Apr 28, 2017 at 5:55 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote: > > Hi, > > > > On Fri, Apr 28, 2017 at 6:19 AM, Muhammad Faiz <mfc...@gmail.com> wrote: > >> > >> So, all frames and errors are correctly reported in order. > >> Also limit the numbers of error during draining to prevent infinite loop. > >> > >> This fix fate failure with THREADS>=4: > >> make fate-h264-attachment-631 THREADS=4 > >> This also reverts a755b725ec1d657609c8bd726ce37e7cf193d03f. > >> > >> Suggested-by: wm4, Ronald S. Bultje, Marton Balint > >> Signed-off-by: Muhammad Faiz <mfc...@gmail.com> > >> --- > >> libavcodec/decode.c | 21 +++++++++++++++++++-- > >> libavcodec/internal.h | 3 +++ > >> libavcodec/pthread_frame.c | 15 +++++++-------- > >> 3 files changed, 29 insertions(+), 10 deletions(-) > >> > >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >> index 6ff3c40..edfae55 100644 > >> --- a/libavcodec/decode.c > >> +++ b/libavcodec/decode.c > >> @@ -568,8 +568,24 @@ FF_ENABLE_DEPRECATION_WARNINGS > >> avctx->time_base = av_inv_q(av_mul_q(avctx->framerate, > >> (AVRational){avctx->ticks_per_frame, 1})); > >> #endif > >> > >> - if (avctx->internal->draining && !got_frame) > >> - avci->draining_done = 1; > >> + /* do not stop draining when got_frame != 0 or ret < 0 */ > >> + if (avctx->internal->draining && !got_frame) { > >> + if (ret < 0) { > >> + /* prevent infinite loop if a decoder wrongly always return > >> error on draining */ > >> + /* reasonable nb_errors_max = maximum b frames + thread count > >> */ > >> + int nb_errors_max = 20 + (HAVE_THREADS && > >> avctx->active_thread_type & FF_THREAD_FRAME ? > >> + avctx->thread_count : 1); > >> + > >> + if (avci->nb_draining_errors++ >= nb_errors_max) { > >> + av_log(avctx, AV_LOG_ERROR, "Too many errors when > >> draining, this is a bug. " > >> + "Stop draining and force EOF.\n"); > >> + avci->draining_done = 1; > >> + ret = AVERROR_BUG; > >> + } > >> + } else { > >> + avci->draining_done = 1; > >> + } > >> + } > > > > > > Hm... I guess this is OK, it would be really nice to have a way of breaking > > in developer builds (e.g. av_assert or so, although I guess technically this > > could be enabled in prod builds also). > > Add av_assert2(). > > > > > Also, Marton suggested to return AVERROR_EOF, maybe handle that here also in > > addition to ret=0? > > Modified. > > Updated patch attached. > > Thank's
> decode.c | 23 +++++++++++++++++++++-- > internal.h | 3 +++ > pthread_frame.c | 15 +++++++-------- > 3 files changed, 31 insertions(+), 10 deletions(-) > d3049c52598070baa9566fc98a089111732595fa > 0001-avcodec-pthread_frame-decode-allow-errors-to-happen-.patch > From f684770e016fa36d458d08383065815882cbc7f8 Mon Sep 17 00:00:00 2001 > From: Muhammad Faiz <mfc...@gmail.com> > Date: Fri, 28 Apr 2017 17:08:39 +0700 > Subject: [PATCH v3] avcodec/pthread_frame, decode: allow errors to happen on > draining > > So, all frames and errors are correctly reported in order. > Also limit the number of errors during draining to prevent infinite loop. > Also return AVERROR_EOF directly on EOF instead of only setting draining_done. > > This fix fate failure with THREADS>=4: > make fate-h264-attachment-631 THREADS=4 > This also reverts a755b725ec1d657609c8bd726ce37e7cf193d03f. > > Suggested-by: wm4, Ronald S. Bultje, Marton Balint > Signed-off-by: Muhammad Faiz <mfc...@gmail.com> > --- > libavcodec/decode.c | 23 +++++++++++++++++++++-- > libavcodec/internal.h | 3 +++ > libavcodec/pthread_frame.c | 15 +++++++-------- > 3 files changed, 31 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 6ff3c40..fb4d4af 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -568,8 +568,26 @@ FF_ENABLE_DEPRECATION_WARNINGS > avctx->time_base = av_inv_q(av_mul_q(avctx->framerate, > (AVRational){avctx->ticks_per_frame, 1})); > #endif > > - if (avctx->internal->draining && !got_frame) > - avci->draining_done = 1; > + /* do not stop draining when got_frame != 0 or ret < 0 */ > + if (avctx->internal->draining && !got_frame) { > + if (ret < 0) { > + /* prevent infinite loop if a decoder wrongly always return > error on draining */ > + /* reasonable nb_errors_max = maximum b frames + thread count */ > + int nb_errors_max = 20 + (HAVE_THREADS && > avctx->active_thread_type & FF_THREAD_FRAME ? > + avctx->thread_count : 1); > + > + if (avci->nb_draining_errors++ >= nb_errors_max) { > + av_log(avctx, AV_LOG_ERROR, "Too many errors when draining, > this is a bug. " > + "Stop draining and force EOF.\n"); > + avci->draining_done = 1; > + ret = AVERROR_BUG; > + av_assert2(0); Please build with --assert-level=2 this triggers for the 2 files i sent you yesterday [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel