[FFmpeg-devel] [PATCH] add init/uninit code to cmdutils.c

[Michael Niedermayer]

On Sat, Sep 25, 2010 at 03:52:50PM +0200, Reimar D?ffinger wrote:
> On Sat, Sep 25, 2010 at 02:26:09PM +0100, M?ns Rullg?rd wrote:
> > Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
> > 
> > > On date Saturday 2010-09-25 14:19:55 +0200, Reimar D?ffinger encoded:
> > >> Hello,
> > >> this avoids some code duplication between ffmpeg and ffplay and also frees
> > >> the AVFormatContext::key value.
> > >> The tests pass, but I do not know the code well enough to be 100%
> > >> certain that using SWS_BICUBIC always instead of sws_flags is ok for
> > >> the option parsing.
> > >
> > > [...]
> > >> Index: cmdutils.c
> > >> ===================================================================
> > >> --- cmdutils.c	(revision 25163)
> > >> +++ cmdutils.c	(working copy)
> > >> @@ -56,6 +56,25 @@
> > >>  
> > >>  const int this_year = 2010;
> > >>  
> > >> +void init_opts(void)
> > >> +{
> > >> +    int i;
> > >> +    for (i = 0; i < AVMEDIA_TYPE_NB; i++)
> > >> +        avcodec_opts[i] = avcodec_alloc_context2(i);
> > >> +    avformat_opts = avformat_alloc_context();
> > >> +    sws_opts = sws_getContext(16, 16, 0, 16, 16, 0, SWS_BICUBIC, NULL, NULL, NULL);
> > >> +}
> > >> +
> > >> +void uninit_opts(void)
> > >> +{
> > >> +    int i;
> > >> +    for (i = 0; i < AVMEDIA_TYPE_NB; i++)
> > >> +        av_freep(&avcodec_opts[i]);
> > >
> > >> +    av_freep(&avformat_opts->key);
> > >
> > > Why is AVFormatContext.key special?
> > 
> > It is the only FF_OPT_TYPE_BINARY option.
> > 
> > > Shouldn't we have a function for automatically clean it up?
> > 
> > I suppose a function traversing the list and freeing all
> > FF_OPT_TYPE_BINARY things might make sense.  Would you like to write
> > one?
> 
> It should also free all FF_OPT_TYPE_STRING.
> However you can't traverse the option list since you don't have one.

You surely can enumerate the list of AVOptions
see av_opt_set_defaults2() and av_next_option()


> I can take this one out if you mind the hack, however I at least
> won't be writing a proper solution to this any time soon.
> (the proper solution would probably to free anything in the
> contexts that is a pointer type and marked as allocated by the
> user in the documentation).

I think ultimately all pointer types should be required to be av_malloced
thats the only way by which they all can be cleaned up and defaults be set
without growing gray hairs ...


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101005/0805ae85/attachment.pgp>