[FFmpeg-devel] [PATCH] Fix opt_default()

Michael Niedermayer michaelni
Tue Dec 16 18:24:21 CET 2008


On Tue, Dec 16, 2008 at 09:14:06AM +0100, Stefano Sabatini wrote:
> On date Tuesday 2008-12-16 02:34:51 +0100, Michael Niedermayer encoded:
> > On Tue, Dec 16, 2008 at 01:49:50AM +0100, Stefano Sabatini wrote:
> > > On date Tuesday 2008-12-16 00:31:02 +0100, Stefano Sabatini encoded:
> > > [...]
> > > > > > Index: ffmpeg/libavcodec/opt.h
> > > > > > ===================================================================
> > > > > > --- ffmpeg.orig/libavcodec/opt.h	2008-12-14 23:17:27.000000000 +0100
> > > > > > +++ ffmpeg/libavcodec/opt.h	2008-12-14 23:21:18.000000000 +0100
> > > > > 
> > > > > > @@ -105,6 +105,14 @@
> > > > > >  attribute_deprecated const AVOption *av_set_string(void *obj, const char *name, const char *val);
> > > > > >  
> > > > > >  /**
> > > > > > + * @return a pointer to the AVOption corresponding to the field set or
> > > > > > + * NULL if no matching AVOption exists, or if the value \p val is not
> > > > > > + * valid
> > > > > > + * @see av_set_string3()
> > > > > > + */
> > > > > > +const AVOption *av_set_string2(void *obj, const char *name, const char *val, int alloc);
> > > > > > +
> > > > > 
> > > > > this should be under #ifdefs to remove it with the next major bump
> > > > 
> > > > So I think it should be deprecated and ifdeffed at the same time. See
> > > > the attached patches (unfortunately there is still a warning which I
> > > > think we cannot avoid).
> > > 
> > > No, that wasn't true.
> [...]
> > > Index: ffmpeg/libavcodec/opt.h
> > > ===================================================================
> > > --- ffmpeg.orig/libavcodec/opt.h	2008-12-15 23:33:10.000000000 +0100
> > > +++ ffmpeg/libavcodec/opt.h	2008-12-16 01:14:20.000000000 +0100
> > 
> > > @@ -105,6 +105,14 @@
> > >  attribute_deprecated const AVOption *av_set_string(void *obj, const char *name, const char *val);
> > >  
> > >  /**
> > > + * @return a pointer to the AVOption corresponding to the field set or
> > > + * NULL if no matching AVOption exists, or if the value \p val is not
> > > + * valid
> > > + * @see av_set_string3()
> > > + */
> > > +const AVOption *av_set_string2(void *obj, const char *name, const char *val, int alloc);
> > > +
> > > +/**
> > >   * Sets the field of obj with the given name to value.
> > >   *
> > >   * @param[in] obj A struct whose first element is a pointer to an
> > 
> > where is the #if version?
> 
> I think it is better to follow the path:
> * define the new function;
> * replace all the uses of the old function
> * deprecate + ifdef the old function
> 
> implemented by the patchset.
> 
> And I think that if you want to ifdef a function then you have also to
> deprecate it, so is nicer to first remove the old function use thus
> avoiding warnings.
> 
> But if you prefer the other way (define av_set_string3() *and*
> ifdef/deprecate av_set_string2() at once) I'm fine with it too.

i prefer to mark the old as deprecated when the new is added because
that is the point where it is deprecated, not once it isnt used anywhere
anymore.
the "deprecated" gives a nice reminder so its not forgotten, when its
done seperately it can easily be forgotten.

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/214b63fa/attachment.pgp>



More information about the ffmpeg-devel mailing list