| Message ID | tencent_F69C02E0AC5BE634D380AA34FF3FCD56BB09@qq.com |
|---|---|
| State | Accepted |
| Commit | 99e12b5736bd4a63186a4fc71b69ca3ec0c9fa34 |
| Headers | show |
| Series | [FFmpeg-devel] ffplay: fix autoexit doesn't work in the case of pb->error | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Thu, 27 Aug 2020, Zhao Zhili wrote: > --- > Goto fail will make ffplay exit immediately. I'm not sure > it is the expected behavior. How about just remove the > check on pb->error so decoders can drain normally? I think it is fine as is, if we simply ignored the error, then looping would start at the place of the error. Also probably it is not good practice to keep using an IO context which already had an IO error. > > fftools/ffplay.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index 6c9c041e9a..9ff0425163 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -3028,8 +3028,12 @@ static int read_thread(void *arg) > packet_queue_put_nullpacket(&is->subtitleq, is->subtitle_stream); > is->eof = 1; > } > - if (ic->pb && ic->pb->error) > - break; > + if (ic->pb && ic->pb->error) { > + if (autoexit) > + goto fail; > + else > + break; > + } > SDL_LockMutex(wait_mutex); > SDL_CondWaitTimeout(is->continue_read_thread, wait_mutex, 10); > SDL_UnlockMutex(wait_mutex); LGTM, thanks. Regards, Marton
> On Aug 27, 2020, at 4:20 PM, Marton Balint <cus@passwd.hu> wrote: > > > > On Thu, 27 Aug 2020, Zhao Zhili wrote: > >> --- >> Goto fail will make ffplay exit immediately. I'm not sure >> it is the expected behavior. How about just remove the >> check on pb->error so decoders can drain normally? > > I think it is fine as is, if we simply ignored the error, then looping would start at the place of the error. Also probably it is not good practice to keep using an IO context which already had an IO error. > >> >> fftools/ffplay.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/fftools/ffplay.c b/fftools/ffplay.c >> index 6c9c041e9a..9ff0425163 100644 >> --- a/fftools/ffplay.c >> +++ b/fftools/ffplay.c >> @@ -3028,8 +3028,12 @@ static int read_thread(void *arg) >> packet_queue_put_nullpacket(&is->subtitleq, is->subtitle_stream); >> is->eof = 1; >> } >> - if (ic->pb && ic->pb->error) >> - break; >> + if (ic->pb && ic->pb->error) { >> + if (autoexit) >> + goto fail; >> + else >> + break; >> + } >> SDL_LockMutex(wait_mutex); >> SDL_CondWaitTimeout(is->continue_read_thread, wait_mutex, 10); >> SDL_UnlockMutex(wait_mutex); > > LGTM, thanks. Ping for further review or merge, thanks. > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
On Mon, 7 Sep 2020, Zhao Zhili wrote: > > >> On Aug 27, 2020, at 4:20 PM, Marton Balint <cus@passwd.hu> wrote: >> >> >> >> On Thu, 27 Aug 2020, Zhao Zhili wrote: >> >>> --- >>> Goto fail will make ffplay exit immediately. I'm not sure >>> it is the expected behavior. How about just remove the >>> check on pb->error so decoders can drain normally? >> >> I think it is fine as is, if we simply ignored the error, then looping would start at the place of the error. Also probably it is not good practice to keep using an IO context which already had an IO error. >> >>> >>> fftools/ffplay.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c >>> index 6c9c041e9a..9ff0425163 100644 >>> --- a/fftools/ffplay.c >>> +++ b/fftools/ffplay.c >>> @@ -3028,8 +3028,12 @@ static int read_thread(void *arg) >>> packet_queue_put_nullpacket(&is->subtitleq, is->subtitle_stream); >>> is->eof = 1; >>> } >>> - if (ic->pb && ic->pb->error) >>> - break; >>> + if (ic->pb && ic->pb->error) { >>> + if (autoexit) >>> + goto fail; >>> + else >>> + break; >>> + } >>> SDL_LockMutex(wait_mutex); >>> SDL_CondWaitTimeout(is->continue_read_thread, wait_mutex, 10); >>> SDL_UnlockMutex(wait_mutex); >> >> LGTM, thanks. > > Ping for further review or merge, thanks. Will apply. Thanks, Marton
diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 6c9c041e9a..9ff0425163 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -3028,8 +3028,12 @@ static int read_thread(void *arg) packet_queue_put_nullpacket(&is->subtitleq, is->subtitle_stream); is->eof = 1; } - if (ic->pb && ic->pb->error) - break; + if (ic->pb && ic->pb->error) { + if (autoexit) + goto fail; + else + break; + } SDL_LockMutex(wait_mutex); SDL_CondWaitTimeout(is->continue_read_thread, wait_mutex, 10); SDL_UnlockMutex(wait_mutex);