[FFmpeg-devel] [PATCH 1/2] avio_alloc_context: Allocate a buffer if NULL is passed

[wm4]

On Thu, 22 May 2014 19:06:54 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, May 22, 2014 at 09:17:28AM +0200, wm4 wrote:
> > On Wed, 21 May 2014 22:18:52 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavformat/aviobuf.c |    9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > > index 738459e..e60574d 100644
> > > --- a/libavformat/aviobuf.c
> > > +++ b/libavformat/aviobuf.c
> > > @@ -119,6 +119,15 @@ AVIOContext *avio_alloc_context(
> > >      AVIOContext *s = av_mallocz(sizeof(AVIOContext));
> > >      if (!s)
> > >          return NULL;
> > > +    if (!buffer && !buffer_size)
> > > +        buffer_size = IO_BUFFER_SIZE;
> > > +    if (!buffer) {
> > > +        buffer = av_malloc(buffer_size);
> > > +        if (!buffer) {
> > > +            av_free(s);
> > > +            return NULL;
> > > +        }
> > > +    }
> > >      ffio_init_context(s, buffer, buffer_size, write_flag, opaque,
> > >                    read_packet, write_packet, seek);
> > >      return s;
> > 
> > Is the buffer actually free'd?
> 
> if the user frees it
> do you suggest this should be done automatically ?

How the hell should the user know that he has to free it? That's
completely arbitrary and unexpected, at least if you see this API for
the first time. (Although last time when I was using this API, I was
utterly surprised that lavf wanted to realloc the buffer I passed, yet I
had to free it myself. I'm not quite sure whether it really reallocates
the buffer, but if so, the avio_alloc_context doxygen is both incorrect
and incomplete.)

Unfortunately, it seems there is no API function to free an avio
context, and the user is just expected to av_free it, so I'm not sure
what's the best course of action. Suddenly requiring the user to call
a newly added avio_free_context() instead of av_free doesn't sound nice.

> 
> > I faintly remember that you have to do
> > this manually with custom AVIO contexts.
> 
> yes
> 
> [...]