> On Aug 24, 2018, at 03:44, Hendrik Leppkes <h.lepp...@gmail.com> wrote: > > On Fri, Aug 24, 2018 at 5:01 AM myp...@gmail.com <mailto:myp...@gmail.com> > <myp...@gmail.com <mailto:myp...@gmail.com>> wrote: >> >> On Fri, Aug 24, 2018 at 5:38 AM Rodger Combs <rodger.co...@gmail.com> wrote: >>> >>> We previously could fail to check errors entirely, or misinterpret read >> errors >>> as normal EOFs. >>> --- >>> libavformat/mpegts.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c >>> index 37a6aa8bff..1350213d39 100644 >>> --- a/libavformat/mpegts.c >>> +++ b/libavformat/mpegts.c >>> @@ -2475,6 +2475,8 @@ static int mpegts_resync(AVFormatContext *s, int >> seekback, const uint8_t *curren >>> >>> for (i = 0; i < ts->resync_size; i++) { >>> c = avio_r8(pb); >>> + if (pb->error) >>> + return pb->error; >>> if (avio_feof(pb)) >>> return AVERROR_EOF; >>> if (c == 0x47) { >>> @@ -2498,8 +2500,13 @@ static int read_packet(AVFormatContext *s, uint8_t >> *buf, int raw_packet_size, >>> >>> for (;;) { >>> len = ffio_read_indirect(pb, buf, TS_PACKET_SIZE, data); >>> - if (len != TS_PACKET_SIZE) >>> - return len < 0 ? len : AVERROR_EOF; >>> + if (len != TS_PACKET_SIZE) { >>> + if (len < 0) >>> + return len; >>> + if (pb->error) >>> + return pb->error; >>> + return AVERROR_EOF; >>> + } >>> /* check packet sync byte */ >>> if ((*data)[0] != 0x47) { >>> /* find a new packet start */ >>> @@ -2670,6 +2677,8 @@ static int mpegts_read_header(AVFormatContext *s) >>> /* read the first 8192 bytes to get packet size */ >>> pos = avio_tell(pb); >>> len = avio_read(pb, buf, sizeof(buf)); >>> + if (len < 0) >>> + return len; >>> ts->raw_packet_size = get_packet_size(buf, len); >>> if (ts->raw_packet_size <= 0) { >>> av_log(s, AV_LOG_WARNING, "Could not detect TS packet size, >> defaulting to non-FEC/DVHS\n"); >>> -- >> As I understand, only after avio_xxx return < 0 to check pb->error, now we >> coding like: >> len = avio_xxx(pb); >> if (len < 0) >> return len; >> if (pb->error) >> return pb->error; >> >> It's stranger way for me, consider Unix API read(2), we just check the >> error after -1 is returned >> ( >> http://man7.org/linux/man-pages/man2/read.2.html >> ) >> >> we usually catch the error >> / error >> number like: >> > > The reason for this is to be able to differentiate between EOF and > read errors. In case of a read error, partial data may still be > returned, and any short read is otherwise considered EOF before this > patch. > The alternative to this would be checking pb->eof_reached, which would > do the same thing, just flip the logic on its head.
I'd actually sort of prefer that, since it'd let me call avio_feof() instead of doing a direct member access, but it turns out that eof_reached is set to 1 in error cases as well. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
