On Sun, Apr 30, 2017 at 8:27 PM, wm4 <nfx...@googlemail.com> wrote:
> On Sun, 30 Apr 2017 06:09:18 +0700
> Muhammad Faiz <mfc...@gmail.com> wrote:
>
>> On Sat, Apr 29, 2017 at 6:18 AM, Muhammad Faiz <mfc...@gmail.com> wrote:
>> > 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
>>
>> Applied (PATCH v2)
>
> This one could probably have waited a bit longer.
>
> Thanks for looking into this and fixing it, anyway.
I'm sorry for pushing it too quickly. I refer on
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-March/208718.html.
There are no clear rules, anyway.
Thank's.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
