Marton Balint: > > > On Sat, 18 Jun 2022, Michael Niedermayer wrote: > >> On Fri, Jun 17, 2022 at 09:53:13PM +0200, Marton Balint wrote: >>> >>> >>> On Tue, 8 Feb 2022, Michael Niedermayer wrote: >>> >>>> Fixes: read_frame_internal() which does not return even though both >>>> demuxer and parser do return >>>> Fixes: >>>> 43717/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-5206008287330304 >>>> >>>> >>>> Found-by: continuous fuzzing process >>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >>>> --- >>>> libavformat/demux.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/libavformat/demux.c b/libavformat/demux.c >>>> index ec34b65288..dd42d32710 100644 >>>> --- a/libavformat/demux.c >>>> +++ b/libavformat/demux.c >>>> @@ -1222,11 +1222,15 @@ static int >>>> read_frame_internal(AVFormatContext *s, AVPacket *pkt) >>>> FFFormatContext *const si = ffformatcontext(s); >>>> int ret, got_packet = 0; >>>> AVDictionary *metadata = NULL; >>>> + int empty = 0; >>>> >>>> while (!got_packet && !si->parse_queue.head) { >>>> AVStream *st; >>>> FFStream *sti; >>>> >>>> + if (empty > 1) >>>> + return AVERROR(EAGAIN); >>>> + >>>> /* read next packet */ >>>> ret = ff_read_packet(s, pkt); >>>> if (ret < 0) { >>>> @@ -1317,6 +1321,8 @@ static int read_frame_internal(AVFormatContext >>>> *s, AVPacket *pkt) >>>> } >>>> got_packet = 1; >>>> } else if (st->discard < AVDISCARD_ALL) { >>>> + if (pkt->size == 0) >>>> + empty ++; >>>> if ((ret = parse_packet(s, pkt, pkt->stream_index, 0)) < 0) >>>> return ret; >>>> st->codecpar->sample_rate = sti->avctx->sample_rate; >>>> -- >>>> 2.17.1 >>> >>> Sorry, just noticed this patchset, and it is very hackish. >> >> yes thats why i waited so many month before i applied it >> some patchsets i forget but this i kept pushing away >> >>> >>> For starters the meaning of AVERROR(EAGAIN) for >>> av_read_frame()/read_frame_internal() is not very well defined. >>> Should the >>> user retry immediately? Should the user sleep() sometime and the >>> retry? Can >>> the user expect that a loop of av_read_frame() will eventually return >>> something different than AVERROR(EAGAIN)? >> >> In the context of this problem the idea is to give the user an opertunity >> to do something else in a single threaded environment or error out if >> its taking too long not produingh anything >> >> >>> >>> I am not sure I understand the original issue entirely, but it looks >>> that >>> instead of fixing the component which returns infinite amount of zero >>> sized >>> packets you implemented a check that makes the user get an EAGAIN() >>> on the >>> second zero-sized packet. >> >> IIRC the problem was that the demuxer produced 0 sized packets and >> alot of them >> for a file only a few hundread bytes large. > > AFAIK a demuxer is not supposed to generate 0-sized packets. Or do you > think otherwise?
Is it not possible to use zero-sized packets to clear the screen (i.e. convey the end timestamp of subtitles transmitted earlier)? (The subtitle formats without duration currently use packets with a size > 0 for that, but there is no real reason why this need to be so.) Another reason for zero-sized packets were side-data only packets (maybe not now, but it might be used in the future; after all, we are constantly adding new side-data-types). (This does not mean that I am against your proposed patch.) - Andreas PS: The fact that the parsing API is based upon buffers and not AVPackets btw leads to buggy side-data handling: If a parser introduces delay, then the side-data will not be attached to the correct packet, but to the preceding one. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
