[FFmpeg-devel] [PATCH] pthread_frame: ignore errors during draining

[wm4]

On Tue, 25 Apr 2017 17:29:10 +0700
Muhammad Faiz <mfcc64 at gmail.com> wrote:

> On Tue, Apr 25, 2017 at 4:09 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> > On Tue, Apr 25, 2017 at 4:32 AM, Muhammad Faiz <mfcc64 at gmail.com> wrote:  
> >> On Tue, Apr 25, 2017 at 1:57 AM, wm4 <nfxjfg at googlemail.com> wrote:  
> >>> This is needed to get compatibility with the behavior before the
> >>> recent decode.c restructuring merge, and fixes fate failures with
> >>> this:
> >>>
> >>>   make fate-h264-attachment-631 THREADS=32
> >>>
> >>> For every 4 threads, a frame is dropped at the point at which
> >>> draining is initialized. The problem starts at THREADS=4.
> >>>
> >>> This patch "fixes" it by ignoring errors during draining (if there
> >>> is a frame to be returned), but there is probably a deeper cause
> >>> and/or a better fix for this. I haven't looked closer at this, but
> >>> was asked to send this patch for discussion.
> >>> ---
> >>>  libavcodec/pthread_frame.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> >>> index 13d682842d..e7fa503adf 100644
> >>> --- a/libavcodec/pthread_frame.c
> >>> +++ b/libavcodec/pthread_frame.c
> >>> @@ -548,7 +548,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
> >>>      fctx->next_finished = finished;
> >>>
> >>>      /* return the size of the consumed packet if no error occurred */
> >>> -    if (err >= 0)
> >>> +    if (err >= 0 || (!avpkt->size && *got_picture_ptr))  
> >>
> >> Probably (*got_picture_ptr && p->result >= 0)
> >> So error is not ignored.
> >>  
> >
> > Isnt the entire point of this to ignore errors during draining?  
> 
> wm4 stated that probably there is a better fix, and not ignoring error
> is a better fix.
> My point is that when a frame is retuned, propagate error from the
> thread that return it,
> not from another thread. More clearly
> if (*got_picture_ptr) err = p->result

Can you send a patch for this?