On Thu, Apr 27, 2017 at 2:59 AM, wm4 <nfx...@googlemail.com> wrote: > On Thu, 27 Apr 2017 01:20:53 +0700 > Muhammad Faiz <mfc...@gmail.com> wrote: > >> On Wed, Apr 26, 2017 at 10:34 PM, 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? >> >> The problem is that return<0 and *got_picture_ptr==0 is also >> considered as eof when avpkt->size==0. > > The old public decode API, or the new API? The latter would be about > the wrapper. I remember adding a hack to avoid the situation that > decoders can get "stuck" during draining, not sure if it was > overwritten in a recent merge.
New API, of course. For old api, it depends on who interprets return value and got_frame value. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
