Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: don't treat I/O errors as EOF
> On Nov 28, 2018, at 13:19, Marton Balint wrote: > > > > On Wed, 28 Nov 2018, Rodger Combs wrote: > >> pb->eof_reached is set on error, so we need to check pb->error, >> even after checking pb->eof_reached or avio_feof(pb), or else we >> can end up returning AVERROR_EOF instead of the actual error code. > > Why eof_reached is set in the first place on error? Why does avio_feof() > return nonzero if we are not at the end of file? That is strictly against its > documentation. Looks like it's been like that since at least 2002. It's probably because a lot of code that loops on e.g. avio_r8 checks for EOF, but doesn't explicitly check for error, so if errors didn't set the EOF flag, they'd just loop forever. Either way, that code needs to be updated to return errors properly, so I don't think removing the EOF-on-error behavior is helpful (it'd just make existing problem cases worse). > > Seems like a fine mess, let's think about how we will resolve this in a > generic way. I don't think this can really be resolved generically, since all the places that currently just return AVERROR_EOF in this case will need updating to read the error from AVIOContext regardless. > > Thanks, > Marton > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: don't treat I/O errors as EOF
On Wed, 28 Nov 2018, Rodger Combs wrote: pb->eof_reached is set on error, so we need to check pb->error, even after checking pb->eof_reached or avio_feof(pb), or else we can end up returning AVERROR_EOF instead of the actual error code. Why eof_reached is set in the first place on error? Why does avio_feof() return nonzero if we are not at the end of file? That is strictly against its documentation. Seems like a fine mess, let's think about how we will resolve this in a generic way. Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: don't treat I/O errors as EOF
pb->eof_reached is set on error, so we need to check pb->error, even after checking pb->eof_reached or avio_feof(pb), or else we can end up returning AVERROR_EOF instead of the actual error code. --- libavformat/matroskadec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index d3c9c33720..2774ccabb2 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -789,7 +789,7 @@ static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos) } matroska->done = 1; -return AVERROR_EOF; +return pb->error ? pb->error : AVERROR_EOF; } /* @@ -836,7 +836,7 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, AVIOContext *pb, pos, pos); return pb->error ? pb->error : AVERROR(EIO); } -return AVERROR_EOF; +return pb->error ? pb->error : AVERROR_EOF; } /* get the length of the EBML number */ -- 2.19.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
