[FFmpeg-devel] [PATCH] Some lavf renames

Michael Niedermayer michaelni
Sat Feb 7 14:00:40 CET 2009


On Sat, Feb 07, 2009 at 09:50:42AM +0100, Stefano Sabatini wrote:
> On date Saturday 2009-02-07 03:14:49 +0100, Michael Niedermayer encoded:
> > On Sat, Feb 07, 2009 at 01:10:16AM +0100, Stefano Sabatini wrote:
> > > On date Friday 2009-02-06 01:08:43 +0100, Michael Niedermayer encoded:
> > > > On Thu, Feb 05, 2009 at 11:04:40PM +0100, Stefano Sabatini wrote:
> > > > > On date Saturday 2009-01-31 01:15:42 +0100, Michael Niedermayer encoded:
> > > > > > On Sat, Jan 31, 2009 at 01:08:37AM +0100, Stefano Sabatini wrote:
> > > > > > > On date Thursday 2009-01-22 00:39:31 +0100, Diego Biurrun encoded:
> > > > > > > > On Wed, Jan 21, 2009 at 03:40:52AM +0100, Michael Niedermayer wrote:
> > > > > > > > > On Tue, Jan 20, 2009 at 11:40:37PM +0100, Stefano Sabatini wrote:
> > > > > > > > > > On date Tuesday 2009-01-20 12:09:20 -0800, Baptiste Coudurier encoded:
> > > > > > > > > > > 
> > > > > > > > > > > Ramiro Polla wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > Wasn't there some discussion some time ago about renaming a bunch of
> > > > > > > > > > > > functions? I think if we're going to rename some functions, we might
> > > > > > > > > > > > as well rename all functions to be consistent and break API only once.
> > > > > > > > > > > 
> > > > > > > > > > > Yes, indeed. We will break API during next major dump, this is not going
> > > > > > > > > > > to happen anytime soon.
> > > > > > > > > > 
> > > > > > > > > > I propose also these renames, people can add to the list:
> > > > > > > > > > 
> > > > > > > > > > lavc:
> > > > > > > > > > register_avcodec          -> avcodec_register()
> > > > > > > > > > 
> > > > > > > > > > lavf:
> > > > > > > > > > ff_reduce_index           -> av_reduce_index
> > > > > > > > > > av_register_input_format  -> avformat_register_input
> > > > > > > > > > av_register_output_format -> avformat_register_output
> > > > > > > > > > av_iformat_next           -> avformat_next_input
> > > > > > > > > > av_oformat_next           -> avformat_next_output
> > > > > > > > > 
> > > > > > > > > i abstain from voting for a color but must note that you guys should
> > > > > > > > > also consider that every rename will mean every app that use the function
> > > > > > > > > needs to be updated, not sure if this justifies this cleanup.
> > > > > > > > > Maybe it does but if i where maintaining some app i likely would be
> > > > > > > > > primarely pissed about every rename that i had to deal with ...
> > > > > > > > > But then its no big deal, if people want it, do the rename ...
> > > > > > > > 
> > > > > > > > I'm generally in favor of the rename, just make sure that you do not
> > > > > > > > overlook a few renames.
> > > > > > > 
> > > > > > > Is OK to start with these?
> > > > > > [...]
> > > > > > 
> > > > > > > av_register_all         -> avformat_register_all
> > > > > > 
> > > > > > iam against this one, it is not correct, this calls avcodec_register_all()
> > > > > > internally and thus makingit look avformat specific is not good
> > > > > > 
> > > > > > 
> > > > > > > register_codec          -> avcodec_register
> > > > > > 
> > > > > > iam ok with this one
> > > > > 
> > > > > Patches attached, regards.
> > > > 
> > > > ok
> > > 
> > > Applied.
> > > 
> > > What about these ones?
> > 
> > too tired ...
> > 
> > [...]
> > 
> > > Rename patch for av_alloc_format_context() attached.
> > [...]
> > > @@ -813,12 +813,18 @@
> > >                         AVInputFormat *fmt,
> > >                         int buf_size,
> > >                         AVFormatParameters *ap);
> > > +
> > > +/**
> > > + * @deprecated Use avformat_alloc_context() instead.
> > > + */
> > > +attribute_deprecated AVFormatContext *av_alloc_format_context(void);
> > > +
> > 
> > should be under #if VER
> > 
> > 
> > [...]
> > > @@ -81,3 +81,8 @@
> > >      ic->av_class = &av_format_context_class;
> > >      return ic;
> > >  }
> > > +
> > > +AVFormatContext *av_alloc_format_context(void)
> > > +{
> > > +    return avformat_alloc_context();
> > > +}
> > 
> > should be under #if VER
> 
> This has been discussed before, I think as a rule is best to follow
> this path:
> 1) rename + deprecation
> 2) replacement in the code
> 3) ifversioning of the old symbol
> 
> doing 3) before 2) may result in a break if a major bump happens
> before 2), well this is not going to happen so from a practical point
> of view it makes no difference but I think it's more correct.
> 
> Anyway if you prefer I'll apply this variant.

yes i prefer it, iam strongly against spliting patches in a way that
asks for cruft to be missed
it is very easy to remove the #if when someone bumps the version, it
is not easy to notice such unused code if there is no if and no comment
(gcc will not warn about it because it cant easily find out that it is
 unused)

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20090207/978c09c9/attachment.pgp>



More information about the ffmpeg-devel mailing list