On Wed, 26 Apr 2017 11:34:22 -0400 "Ronald S. Bultje" <rsbul...@gmail.com> wrote:
> Hi, > > On Wed, Apr 26, 2017 at 1:21 AM, Muhammad Faiz <mfc...@gmail.com> wrote: > > > On Wed, Apr 26, 2017 at 4:09 AM, wm4 <nfx...@googlemail.com> wrote: > > > On Tue, 25 Apr 2017 23:52:04 +0700 > > > Muhammad Faiz <mfc...@gmail.com> wrote: > > > > > >> when frame is received, not from other threads. > > >> > > >> Should fix fate failure with THREADS>=4: > > >> make fate-h264-attachment-631 THREADS=4 > > >> > > >> Signed-off-by: Muhammad Faiz <mfc...@gmail.com> > > >> --- > > >> libavcodec/pthread_frame.c | 4 ++++ > > >> 1 file changed, 4 insertions(+) > > >> > > >> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > > >> index 13d6828..c452ed7 100644 > > >> --- a/libavcodec/pthread_frame.c > > >> +++ b/libavcodec/pthread_frame.c > > >> @@ -547,6 +547,10 @@ int ff_thread_decode_frame(AVCodecContext *avctx, > > >> > > >> fctx->next_finished = finished; > > >> > > >> + /* if frame is returned, properly set err from the thread that > > return frame */ > > >> + if (*got_picture_ptr) > > >> + err = p->result; > > >> + > > >> /* return the size of the consumed packet if no error occurred */ > > >> if (err >= 0) > > >> err = avpkt->size; > > > > > > Well, the logic confuses me. Does this override an earlier set err > > > value? > > > > Yes, because an earlier set err value may be from a different thread. > > > > >Could err be set to the correct value in the first place (inside > > > of the loop)? > > > > No, it was intended on 32a5b631267 > > > Thanks for working on this. It's good to get more people familiar with this > code. > > So, I'm looking at understanding this, you're trying to fix the case where > during draining, we may iterate over >1 worker thread cases where the first > returned an error code without having decoded a frame, and the second > decoded a frame without returning an error code, right? The current code > would return a frame with an error return code, which I believe is then > ignored by the user thread. > > So, you're basically trying to say that instead, we should ignore the > error. I agree that fixes the issue of md5 mismatch w/ vs. w/o threads, but > I doubt that it's fundamentally more correct, because the user thread still > misses out on error codes from the worker threads. Wouldn't you agree that > we should - even during draining - not iterate over N threads, but just the > next thread, and return either an error or a decoded frame, and keep doing > that until all worker threads are flushed, which we can then signal e.g. by > return=0 and *got_picture_ptr=0? That actually sounds good to me (although this issue is currently a distraction to me so I never thought too deeply about it, not looked too closely at the code). _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
