[FFmpeg-devel] [PATCH] lavf/mpegts: improve read error handling

[mypopy at gmail.com]

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:

len =  read(fd, buf, buf_size);
if (len < 0)
   handle the error with error number.