[FFmpeg-devel] [PATCH 11/15] avformat/flacdec:Return correct error-codes on read-failure

[Tomas Härdin]

tis 2024-10-29 klockan 18:42 +0100 skrev Michael Niedermayer:
> On Tue, Oct 29, 2024 at 03:50:05PM +0100, Tomas Härdin wrote:
> > Reasonable enough. Might want a sample
> > 
> > Spotify comments
> > ----------------
> > Unexpected EOF is treated as invalid data
> > 
> > /Tomas
> 
> >  flacdec.c |   20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 8cf62c7b8b7e576cc0e2a0a1e49c4f42be5fc254  0011-avformat-flacdec-
> > Return-correct-error-codes-on-read-.patch
> > From 4d803c5f5c13e194a0e52d287f021b73ec7172bd Mon Sep 17 00:00:00
> > 2001
> > From: Ulrik <ulrikm at spotify.com>
> > Date: Thu, 26 Jan 2023 17:51:02 +0100
> > Subject: [PATCH 11/15] avformat/flacdec:Return correct error-codes
> > on
> >  read-failure
> > 
> > Forward errors from `avio_read` directly. When `avio_read` sees EOF
> > before
> > expected bytes can be read, consistently return
> > `AVERROR_INVALIDDATA`
> > 
> > We used to return `AVERROR(AVERROR_INVALIDDATA)` when failing to
> > read
> > metadata block headers. `AVERROR_INVALIDDATA` is already negative,
> > so
> > wrapping in `AVERROR` leads to double-negation.
> > 
> > We used to return `AVERROR(EIO)` when failing to read extended
> > metadata.
> > However, many times, the IO-layer is not at fault, the input data
> > is simply
> > corrupted (truncated), so we return `AVERROR_INVALIDDATA` here as
> > well.
> > ---
> >  libavformat/flacdec.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> > index 9f65c25864..be305fec82 100644
> > --- a/libavformat/flacdec.c
> > +++ b/libavformat/flacdec.c
> > @@ -81,8 +81,15 @@ static int flac_read_header(AVFormatContext *s)
> >  
> >      /* process metadata blocks */
> >      while (!avio_feof(s->pb) && !metadata_last) {
> > -        if (avio_read(s->pb, header, 4) != 4)
> > -            return AVERROR_INVALIDDATA;
> > +        ret = avio_read(s->pb, header, 4);
> 
> > +        if (ret != 4) {
> > +            if (ret < 0) {
> > +                goto fail;
> > +            } else {
> > +                return AVERROR_INVALIDDATA;
> > +            }
> > +        }
> 
> this is wierd
> because, one path goto fails the other returns directly.

Could be a rebase mistake. I'll take a second look at it tomorrow

/Tomas