[FFmpeg-devel] [PATCH 1/3] avformat/avio: add resizeable field to AVIOContext

[Michael Niedermayer]

On Tue, Apr 21, 2015 at 02:41:15PM +0200, wm4 wrote:
> On Tue, 21 Apr 2015 14:37:36 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Tue, Apr 21, 2015 at 01:52:05PM +0200, wm4 wrote:
> > > On Tue, 21 Apr 2015 13:22:00 +0200
> > > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > 
> > > > This indicates that its safe to use av_free/av_malloc on the IO context
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > > ---
> > > >  libavformat/avio.h    |    7 +++++++
> > > >  libavformat/aviobuf.c |    1 +
> > > >  libavformat/segment.c |    1 +
> > > >  libavformat/wtvdec.c  |    3 ++-
> > > >  4 files changed, 11 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavformat/avio.h b/libavformat/avio.h
> > > > index 51913e3..73d1645 100644
> > > > --- a/libavformat/avio.h
> > > > +++ b/libavformat/avio.h
> > > > @@ -196,6 +196,13 @@ typedef struct AVIOContext {
> > > >       * This field is internal to libavformat and access from outside is not allowed.
> > > >       */
> > > >      int orig_buffer_size;
> > > > +
> > > > +    /**
> > > > +     * The io buffer can be resized or freed with av_free / av_malloc.
> > > > +     * The user application does not keep a private copy of the buffer pointer
> > > > +     * which would become stale on such reallocation.
> > > > +     */
> > > > +    int resizeable;
> > > >  } AVIOContext;
> > > >  
> > > >  /* unbuffered I/O */
> > > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > > > index 7de59f1..b32ff9f 100644
> > > > --- a/libavformat/aviobuf.c
> > > > +++ b/libavformat/aviobuf.c
> > > > @@ -793,6 +793,7 @@ int ffio_fdopen(AVIOContext **s, URLContext *h)
> > > >          (*s)->read_seek  = (int64_t (*)(void *, int, int64_t, int))h->prot->url_read_seek;
> > > >      }
> > > >      (*s)->av_class = &ff_avio_class;
> > > > +    (*s)->resizeable = 1;
> > > >      return 0;
> > > >  }
> > > >  
> > > > diff --git a/libavformat/segment.c b/libavformat/segment.c
> > > > index 1162ea2..6504b46 100644
> > > > --- a/libavformat/segment.c
> > > > +++ b/libavformat/segment.c
> > > > @@ -511,6 +511,7 @@ static int open_null_ctx(AVIOContext **ctx)
> > > >          av_free(buf);
> > > >          return AVERROR(ENOMEM);
> > > >      }
> > > > +    (*ctx)->resizeable = 1;
> > > >      return 0;
> > > >  }
> > > >  
> > > > diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> > > > index e226690..7b5477b 100644
> > > > --- a/libavformat/wtvdec.c
> > > > +++ b/libavformat/wtvdec.c
> > > > @@ -243,7 +243,8 @@ static AVIOContext * wtvfile_open_sector(int first_sector, uint64_t length, int
> > > >          av_freep(&buffer);
> > > >          av_freep(&wf->sectors);
> > > >          av_freep(&wf);
> > > > -    }
> > > > +    } else
> > > > +        pb->resizeable = 1;
> > > >      return pb;
> > > >  }
> > > >  
> > > 
> > > Isn't it already required by default that AVIOContext might resize the
> > > buffer?
> > 
> > technically yes, but theres alot of code, well possibly most code
> > using it that gets this wrong (videolan is one) and the consequences
> > are rather bad (double free and such)
> > so i think its better i we dont assume that the buffer can be resized
> > by default
> > 
> > [...]
> > 
> 
> That comes a bit late. Wouldn't it be better to free the user from the
> responsibility of allocating this buffer entirely?

yes this was easily possible through the use of a custom URLProtocol
and as far as my oppinion is concerned, i do want URLProtocol and
all other things like AVFilters AVIn/OutputFormats, ... to be
creatable by user applications ...

Also simple user apps dont need to create the buffer anyway, ffmpeg*c
ffplay*c and most of our examples do not do. But that limits them
to the existing URLProtocols for IO which is a problem for many user
apps i think

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150421/5c78c977/attachment.asc>