diff mbox series

[FFmpeg-devel] lavf: return AVERROR_EOF rather than EIO on EOF

Message ID 20230327080023.28400-1-anton@khirnov.net
State Accepted
Commit 3f4c41839d5d3d02a12cc6a7407e6586996040e7
Headers show
Series [FFmpeg-devel] lavf: return AVERROR_EOF rather than EIO on EOF | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

Anton Khirnov March 27, 2023, 8 a.m. UTC
---
 libavformat/anm.c          | 2 +-
 libavformat/dauddec.c      | 2 +-
 libavformat/filmstripdec.c | 2 +-
 libavformat/idroqdec.c     | 2 +-
 libavformat/sol.c          | 3 ++-
 libavformat/vc1test.c      | 2 +-
 6 files changed, 7 insertions(+), 6 deletions(-)

Comments

Paul B Mahol March 27, 2023, 8:27 a.m. UTC | #1
LGTM
Marton Balint March 27, 2023, 5:42 p.m. UTC | #2
On Mon, 27 Mar 2023, Anton Khirnov wrote:

> ---
> libavformat/anm.c          | 2 +-
> libavformat/dauddec.c      | 2 +-
> libavformat/filmstripdec.c | 2 +-
> libavformat/idroqdec.c     | 2 +-
> libavformat/sol.c          | 3 ++-
> libavformat/vc1test.c      | 2 +-
> 6 files changed, 7 insertions(+), 6 deletions(-)

Aren't these supposed to return AVERROR_INVALIDDATA? Beacuse these are 
most likely premature EOF-s when the file is truncated.

Regards,
Marton

>
> diff --git a/libavformat/anm.c b/libavformat/anm.c
> index 7feba4ed1e7..f2ac6958a9a 100644
> --- a/libavformat/anm.c
> +++ b/libavformat/anm.c
> @@ -172,7 +172,7 @@ static int read_packet(AVFormatContext *s,
>     int tmp, record_size;
>
>     if (avio_feof(s->pb))
> -        return AVERROR(EIO);
> +        return AVERROR_EOF;
>
>     if (anm->page < 0)
>         return anm->page;
> diff --git a/libavformat/dauddec.c b/libavformat/dauddec.c
> index dbbd39a3b40..0cbf7e4e368 100644
> --- a/libavformat/dauddec.c
> +++ b/libavformat/dauddec.c
> @@ -41,7 +41,7 @@ static int daud_packet(AVFormatContext *s, AVPacket *pkt) {
>     AVIOContext *pb = s->pb;
>     int ret, size;
>     if (avio_feof(pb))
> -        return AVERROR(EIO);
> +        return AVERROR_EOF;
>     size = avio_rb16(pb);
>     avio_rb16(pb); // unknown
>     ret = av_get_packet(pb, pkt, size);
> diff --git a/libavformat/filmstripdec.c b/libavformat/filmstripdec.c
> index 2b6ba63fcf0..000f807181d 100644
> --- a/libavformat/filmstripdec.c
> +++ b/libavformat/filmstripdec.c
> @@ -86,7 +86,7 @@ static int read_packet(AVFormatContext *s,
>     AVStream *st = s->streams[0];
>
>     if (avio_feof(s->pb))
> -        return AVERROR(EIO);
> +        return AVERROR_EOF;
>     pkt->dts = avio_tell(s->pb) / (st->codecpar->width * (int64_t)(st->codecpar->height + film->leading) * 4);
>     pkt->size = av_get_packet(s->pb, pkt, st->codecpar->width * st->codecpar->height * 4);
>     avio_skip(s->pb, st->codecpar->width * (int64_t) film->leading * 4);
> diff --git a/libavformat/idroqdec.c b/libavformat/idroqdec.c
> index c9fc972780a..01ea2bb77ba 100644
> --- a/libavformat/idroqdec.c
> +++ b/libavformat/idroqdec.c
> @@ -107,7 +107,7 @@ static int roq_read_packet(AVFormatContext *s,
>     while (!packet_read) {
>
>         if (avio_feof(s->pb))
> -            return AVERROR(EIO);
> +            return AVERROR_EOF;
>
>         /* get the next chunk preamble */
>         if ((ret = avio_read(pb, preamble, RoQ_CHUNK_PREAMBLE_SIZE)) !=
> diff --git a/libavformat/sol.c b/libavformat/sol.c
> index b92cfb36fee..a276642728c 100644
> --- a/libavformat/sol.c
> +++ b/libavformat/sol.c
> @@ -127,7 +127,8 @@ static int sol_read_packet(AVFormatContext *s,
>     int ret;
>
>     if (avio_feof(s->pb))
> -        return AVERROR(EIO);
> +        return AVERROR_EOF;
> +
>     ret= av_get_packet(s->pb, pkt, MAX_SIZE);
>     if (ret < 0)
>         return ret;
> diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
> index 67edc699c5f..f63ffee69bb 100644
> --- a/libavformat/vc1test.c
> +++ b/libavformat/vc1test.c
> @@ -101,7 +101,7 @@ static int vc1t_read_packet(AVFormatContext *s,
>     uint32_t pts;
>
>     if(avio_feof(pb))
> -        return AVERROR(EIO);
> +        return AVERROR_EOF;
>
>     frame_size = avio_rl24(pb);
>     if(avio_r8(pb) & 0x80)
> -- 
> 2.39.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Marton Balint March 27, 2023, 7:08 p.m. UTC | #3
On Mon, 27 Mar 2023, Marton Balint wrote:

>
>
> On Mon, 27 Mar 2023, Anton Khirnov wrote:
>
>>  ---
>>  libavformat/anm.c          | 2 +-
>>  libavformat/dauddec.c      | 2 +-
>>  libavformat/filmstripdec.c | 2 +-
>>  libavformat/idroqdec.c     | 2 +-
>>  libavformat/sol.c          | 3 ++-
>>  libavformat/vc1test.c      | 2 +-
>>  6 files changed, 7 insertions(+), 6 deletions(-)
>
> Aren't these supposed to return AVERROR_INVALIDDATA? Beacuse these are most 
> likely premature EOF-s when the file is truncated.

Disregard my comment, I can see now that these are all in the 
beginning of read_packet().

Regards,
Marton



>
> Regards,
> Marton
>
>>
>>  diff --git a/libavformat/anm.c b/libavformat/anm.c
>>  index 7feba4ed1e7..f2ac6958a9a 100644
>>  --- a/libavformat/anm.c
>>  +++ b/libavformat/anm.c
>> @@  -172,7 +172,7 @@ static int read_packet(AVFormatContext *s,
>>      int tmp, record_size;
>>
>>      if (avio_feof(s->pb))
>>  -        return AVERROR(EIO);
>>  +        return AVERROR_EOF;
>>
>>      if (anm->page < 0)
>>          return anm->page;
>>  diff --git a/libavformat/dauddec.c b/libavformat/dauddec.c
>>  index dbbd39a3b40..0cbf7e4e368 100644
>>  --- a/libavformat/dauddec.c
>>  +++ b/libavformat/dauddec.c
>> @@  -41,7 +41,7 @@ static int daud_packet(AVFormatContext *s, AVPacket 
>> @@  *pkt) {
>>      AVIOContext *pb = s->pb;
>>      int ret, size;
>>      if (avio_feof(pb))
>>  -        return AVERROR(EIO);
>>  +        return AVERROR_EOF;
>>      size = avio_rb16(pb);
>>      avio_rb16(pb); // unknown
>>      ret = av_get_packet(pb, pkt, size);
>>  diff --git a/libavformat/filmstripdec.c b/libavformat/filmstripdec.c
>>  index 2b6ba63fcf0..000f807181d 100644
>>  --- a/libavformat/filmstripdec.c
>>  +++ b/libavformat/filmstripdec.c
>> @@  -86,7 +86,7 @@ static int read_packet(AVFormatContext *s,
>>      AVStream *st = s->streams[0];
>>
>>      if (avio_feof(s->pb))
>>  -        return AVERROR(EIO);
>>  +        return AVERROR_EOF;
>>      pkt->dts = avio_tell(s->pb) / (st->codecpar->width *
>>      (int64_t)(st->codecpar->height + film->leading) * 4);
>>      pkt->size = av_get_packet(s->pb, pkt, st->codecpar->width *
>>      st->codecpar->height * 4);
>>      avio_skip(s->pb, st->codecpar->width * (int64_t) film->leading * 4);
>>  diff --git a/libavformat/idroqdec.c b/libavformat/idroqdec.c
>>  index c9fc972780a..01ea2bb77ba 100644
>>  --- a/libavformat/idroqdec.c
>>  +++ b/libavformat/idroqdec.c
>> @@  -107,7 +107,7 @@ static int roq_read_packet(AVFormatContext *s,
>>      while (!packet_read) {
>>
>>          if (avio_feof(s->pb))
>>  -            return AVERROR(EIO);
>>  +            return AVERROR_EOF;
>>
>>          /* get the next chunk preamble */
>>          if ((ret = avio_read(pb, preamble, RoQ_CHUNK_PREAMBLE_SIZE)) !=
>>  diff --git a/libavformat/sol.c b/libavformat/sol.c
>>  index b92cfb36fee..a276642728c 100644
>>  --- a/libavformat/sol.c
>>  +++ b/libavformat/sol.c
>> @@  -127,7 +127,8 @@ static int sol_read_packet(AVFormatContext *s,
>>      int ret;
>>
>>      if (avio_feof(s->pb))
>>  -        return AVERROR(EIO);
>>  +        return AVERROR_EOF;
>>  +
>>      ret= av_get_packet(s->pb, pkt, MAX_SIZE);
>>      if (ret < 0)
>>          return ret;
>>  diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
>>  index 67edc699c5f..f63ffee69bb 100644
>>  --- a/libavformat/vc1test.c
>>  +++ b/libavformat/vc1test.c
>> @@  -101,7 +101,7 @@ static int vc1t_read_packet(AVFormatContext *s,
>>      uint32_t pts;
>>
>>      if(avio_feof(pb))
>>  -        return AVERROR(EIO);
>>  +        return AVERROR_EOF;
>>
>>      frame_size = avio_rl24(pb);
>>      if(avio_r8(pb) & 0x80)
>>  --
>>  2.39.1
>>
>>  _______________________________________________
>>  ffmpeg-devel mailing list
>>  ffmpeg-devel@ffmpeg.org
>>  https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>  To unsubscribe, visit link above, or email
>>  ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Anton Khirnov March 27, 2023, 7:22 p.m. UTC | #4
Quoting Marton Balint (2023-03-27 21:08:54)
> 
> 
> On Mon, 27 Mar 2023, Marton Balint wrote:
> 
> >
> >
> > On Mon, 27 Mar 2023, Anton Khirnov wrote:
> >
> >>  ---
> >>  libavformat/anm.c          | 2 +-
> >>  libavformat/dauddec.c      | 2 +-
> >>  libavformat/filmstripdec.c | 2 +-
> >>  libavformat/idroqdec.c     | 2 +-
> >>  libavformat/sol.c          | 3 ++-
> >>  libavformat/vc1test.c      | 2 +-
> >>  6 files changed, 7 insertions(+), 6 deletions(-)
> >
> > Aren't these supposed to return AVERROR_INVALIDDATA? Beacuse these are most 
> > likely premature EOF-s when the file is truncated.
> 
> Disregard my comment, I can see now that these are all in the 
> beginning of read_packet().

Yeah, I picked just those where it was clear that it's an actual EOF.
There's a bunch of other EIO uses left that should probably be
INVALIDDATA, watches pelcome.
diff mbox series

Patch

diff --git a/libavformat/anm.c b/libavformat/anm.c
index 7feba4ed1e7..f2ac6958a9a 100644
--- a/libavformat/anm.c
+++ b/libavformat/anm.c
@@ -172,7 +172,7 @@  static int read_packet(AVFormatContext *s,
     int tmp, record_size;
 
     if (avio_feof(s->pb))
-        return AVERROR(EIO);
+        return AVERROR_EOF;
 
     if (anm->page < 0)
         return anm->page;
diff --git a/libavformat/dauddec.c b/libavformat/dauddec.c
index dbbd39a3b40..0cbf7e4e368 100644
--- a/libavformat/dauddec.c
+++ b/libavformat/dauddec.c
@@ -41,7 +41,7 @@  static int daud_packet(AVFormatContext *s, AVPacket *pkt) {
     AVIOContext *pb = s->pb;
     int ret, size;
     if (avio_feof(pb))
-        return AVERROR(EIO);
+        return AVERROR_EOF;
     size = avio_rb16(pb);
     avio_rb16(pb); // unknown
     ret = av_get_packet(pb, pkt, size);
diff --git a/libavformat/filmstripdec.c b/libavformat/filmstripdec.c
index 2b6ba63fcf0..000f807181d 100644
--- a/libavformat/filmstripdec.c
+++ b/libavformat/filmstripdec.c
@@ -86,7 +86,7 @@  static int read_packet(AVFormatContext *s,
     AVStream *st = s->streams[0];
 
     if (avio_feof(s->pb))
-        return AVERROR(EIO);
+        return AVERROR_EOF;
     pkt->dts = avio_tell(s->pb) / (st->codecpar->width * (int64_t)(st->codecpar->height + film->leading) * 4);
     pkt->size = av_get_packet(s->pb, pkt, st->codecpar->width * st->codecpar->height * 4);
     avio_skip(s->pb, st->codecpar->width * (int64_t) film->leading * 4);
diff --git a/libavformat/idroqdec.c b/libavformat/idroqdec.c
index c9fc972780a..01ea2bb77ba 100644
--- a/libavformat/idroqdec.c
+++ b/libavformat/idroqdec.c
@@ -107,7 +107,7 @@  static int roq_read_packet(AVFormatContext *s,
     while (!packet_read) {
 
         if (avio_feof(s->pb))
-            return AVERROR(EIO);
+            return AVERROR_EOF;
 
         /* get the next chunk preamble */
         if ((ret = avio_read(pb, preamble, RoQ_CHUNK_PREAMBLE_SIZE)) !=
diff --git a/libavformat/sol.c b/libavformat/sol.c
index b92cfb36fee..a276642728c 100644
--- a/libavformat/sol.c
+++ b/libavformat/sol.c
@@ -127,7 +127,8 @@  static int sol_read_packet(AVFormatContext *s,
     int ret;
 
     if (avio_feof(s->pb))
-        return AVERROR(EIO);
+        return AVERROR_EOF;
+
     ret= av_get_packet(s->pb, pkt, MAX_SIZE);
     if (ret < 0)
         return ret;
diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
index 67edc699c5f..f63ffee69bb 100644
--- a/libavformat/vc1test.c
+++ b/libavformat/vc1test.c
@@ -101,7 +101,7 @@  static int vc1t_read_packet(AVFormatContext *s,
     uint32_t pts;
 
     if(avio_feof(pb))
-        return AVERROR(EIO);
+        return AVERROR_EOF;
 
     frame_size = avio_rl24(pb);
     if(avio_r8(pb) & 0x80)