Message ID | 20170430070433.25176-1-mfcc64@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Sun, 30 Apr 2017 14:04:33 +0700 Muhammad Faiz <mfcc64@gmail.com> wrote: > Fix fate failures: > make fate-mov THREADS=32 > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavcodec/decode.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index edfae55..6ec423b 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -557,9 +557,6 @@ FF_ENABLE_DEPRECATION_WARNINGS > avci->showed_multi_packet_warning = 1; > } > > - if (!got_frame) > - av_frame_unref(frame); > - > if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) > ret = pkt->size; > > @@ -568,8 +565,9 @@ FF_ENABLE_DEPRECATION_WARNINGS > avctx->time_base = av_inv_q(av_mul_q(avctx->framerate, (AVRational){avctx->ticks_per_frame, 1})); > #endif > > - /* do not stop draining when got_frame != 0 or ret < 0 */ > - if (avctx->internal->draining && !got_frame) { > + /* do not stop draining when frame->buf[0] != NULL or ret < 0 */ > + /* at this point, got_frame == 0 when discarding frame */ Does that means the internal API can return got_frame=0 but frame->buf[0] set or what? That seems like chaos. They're never supposed to mismatch, and I think the av_frame_unref() you moved is supposed to keep this consistent to help decoders which allocated a frame, but did not successfully decode anything to it. > + if (avctx->internal->draining && !frame->buf[0]) { > if (ret < 0) { > /* prevent infinite loop if a decoder wrongly always return error on draining */ > /* reasonable nb_errors_max = maximum b frames + thread count */ > @@ -603,7 +601,9 @@ FF_ENABLE_DEPRECATION_WARNINGS > avci->last_pkt_props->dts = AV_NOPTS_VALUE; > } > > - if (got_frame) > + if (!got_frame) > + av_frame_unref(frame); > + else > av_assert0(frame->buf[0]); > > return ret < 0 ? ret : 0;
On Sun, Apr 30, 2017 at 8:31 PM, wm4 <nfxjfg@googlemail.com> wrote: > On Sun, 30 Apr 2017 14:04:33 +0700 > Muhammad Faiz <mfcc64@gmail.com> wrote: > >> Fix fate failures: >> make fate-mov THREADS=32 >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libavcodec/decode.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >> index edfae55..6ec423b 100644 >> --- a/libavcodec/decode.c >> +++ b/libavcodec/decode.c >> @@ -557,9 +557,6 @@ FF_ENABLE_DEPRECATION_WARNINGS >> avci->showed_multi_packet_warning = 1; >> } >> >> - if (!got_frame) >> - av_frame_unref(frame); >> - >> if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) >> ret = pkt->size; >> >> @@ -568,8 +565,9 @@ FF_ENABLE_DEPRECATION_WARNINGS >> avctx->time_base = av_inv_q(av_mul_q(avctx->framerate, (AVRational){avctx->ticks_per_frame, 1})); >> #endif >> >> - /* do not stop draining when got_frame != 0 or ret < 0 */ >> - if (avctx->internal->draining && !got_frame) { >> + /* do not stop draining when frame->buf[0] != NULL or ret < 0 */ >> + /* at this point, got_frame == 0 when discarding frame */ > > Does that means the internal API can return got_frame=0 but > frame->buf[0] set or what? That seems like chaos. They're never No. got_frame is unset in decode_simple_internal() when the frame has AV_FRAME_FLAG_DISCARD. > supposed to mismatch, and I think the av_frame_unref() you moved > is supposed to keep this consistent to help decoders which allocated > a frame, but did not successfully decode anything to it. I moved av_frame_unref() to make frame->buf[0] available for eof testing. > >> + if (avctx->internal->draining && !frame->buf[0]) { >> if (ret < 0) { >> /* prevent infinite loop if a decoder wrongly always return error on draining */ >> /* reasonable nb_errors_max = maximum b frames + thread count */ >> @@ -603,7 +601,9 @@ FF_ENABLE_DEPRECATION_WARNINGS >> avci->last_pkt_props->dts = AV_NOPTS_VALUE; >> } >> >> - if (got_frame) >> + if (!got_frame) >> + av_frame_unref(frame); >> + else >> av_assert0(frame->buf[0]); >> >> return ret < 0 ? ret : 0; > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Sun, Apr 30, 2017 at 02:04:33PM +0700, Muhammad Faiz wrote: > Fix fate failures: > make fate-mov THREADS=32 > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavcodec/decode.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) breaks (infinit loop it seems) ./ffmpeg_g -i ~/tickets//1873/zygoa8k_q0.mov -f null - http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket1873/ [...]
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index edfae55..6ec423b 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -557,9 +557,6 @@ FF_ENABLE_DEPRECATION_WARNINGS avci->showed_multi_packet_warning = 1; } - if (!got_frame) - av_frame_unref(frame); - if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) ret = pkt->size; @@ -568,8 +565,9 @@ FF_ENABLE_DEPRECATION_WARNINGS avctx->time_base = av_inv_q(av_mul_q(avctx->framerate, (AVRational){avctx->ticks_per_frame, 1})); #endif - /* do not stop draining when got_frame != 0 or ret < 0 */ - if (avctx->internal->draining && !got_frame) { + /* do not stop draining when frame->buf[0] != NULL or ret < 0 */ + /* at this point, got_frame == 0 when discarding frame */ + if (avctx->internal->draining && !frame->buf[0]) { if (ret < 0) { /* prevent infinite loop if a decoder wrongly always return error on draining */ /* reasonable nb_errors_max = maximum b frames + thread count */ @@ -603,7 +601,9 @@ FF_ENABLE_DEPRECATION_WARNINGS avci->last_pkt_props->dts = AV_NOPTS_VALUE; } - if (got_frame) + if (!got_frame) + av_frame_unref(frame); + else av_assert0(frame->buf[0]); return ret < 0 ? ret : 0;
Fix fate failures: make fate-mov THREADS=32 Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libavcodec/decode.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)