[FFmpeg-devel] [PATCH] lavf/mpegts: improve read error handling
Hendrik Leppkes
h.leppkes at gmail.com
Fri Aug 24 11:44:35 EEST 2018
On Fri, Aug 24, 2018 at 5:01 AM mypopy at gmail.com <mypopy at gmail.com> wrote:
>
> On Fri, Aug 24, 2018 at 5:38 AM Rodger Combs <rodger.combs at 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.
- Hendrik
More information about the ffmpeg-devel
mailing list