On Sat, Apr 29, 2017 at 6:01 AM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:
> 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
I've already dropped the updated patch.
Thank's
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
