[FFmpeg-devel] [PATCH 3/6] libavformat: Add a function for freeing an AVFormatContext

[Martin Storsjö]

On Thu, 3 Feb 2011, Stefano Sabatini wrote:

> On date Thursday 2011-02-03 14:32:25 +0200, Martin Storsj? encoded:
> > On Thu, 3 Feb 2011, Anton Khirnov wrote:
> > 
> > > On Thu, Feb 03, 2011 at 02:10:14PM +0200, Martin Storsj? wrote:
> > > > This function is useful for freeing data structures allocated by
> > > > muxers, which currently have to be freed manually by the caller.
> > > > ---
> > > >  libavformat/avformat.h |    6 ++++++
> > > >  libavformat/utils.c    |   11 ++++++++---
> > > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > > > index f9f9be5..1cbe274 100644
> > > > --- a/libavformat/avformat.h
> > > > +++ b/libavformat/avformat.h
> > > > @@ -1230,6 +1230,12 @@ void av_close_input_stream(AVFormatContext *s);
> > > >  void av_close_input_file(AVFormatContext *s);
> > > >  
> > > >  /**
> > > > + * Free an AVFormatContext and all its streams.
> > > > + * @param s context to free
> > > > + */
> > > > +void av_free_context(AVFormatContext *s);
> > > > +
> > > I think avformat_free_context would be a better name, because:
> > > 1) it's consistent with existing avformat_alloc_context
> > > 2) av_free_context doesn't tell you that you're freeing a _format_
> > > context; it might also conflict with future similar functions in
> > > lavc and friends.
> > 
> > Good point. Any others in favour of avformat_free_context, or preferring 
> > any other versions?
> 
> avformat_free_context looks fine. BTW, what about to make the function
> takes **s and thus set to NULL the pointed variable?
> 
> I mean:
> void avformat_free_context(AVFormatContext **s);
> 
> then have:
> AVFormatContext *fmtctx;
> ...
> avformat_free_context(&fmtctx);
> 
> rahter than:
> avformat_free_context(fmtctx);
> fmtctx = NULL;
> 
> That's just an idea, consider consistency with the rest of the API...

Most of the other similar functions doesn't do this, except for 
av_freep(). Also, in the places where I use this in patch #4, only one of 
the would benefit from it, the other ones would just zero out a local 
variable...

// Martin