On Sat, Apr 29, 2017 at 6:18 AM, Muhammad Faiz wrote:
> On Sat, Apr 29, 2017 at 6:01 AM, Michael Niedermayer
> 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
>>> wrote:
>>> > Hi,
>>> >
>>> > On Fri, Apr 28, 2017 at 6:19 AM, Muhammad Faiz 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
>>> >> ---
>>> >> 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
>>> 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
>>> ---
>>> 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_T
