[FFmpeg-devel] [PATCH] lavfi: consistently use int for sample_rate in AVFilterLink and AVFilterBufferRefAudioProps

[Michael Niedermayer]

On Tue, Aug 23, 2011 at 01:29:58AM +0200, Stefano Sabatini wrote:
> On date Monday 2011-08-22 15:46:13 +0200, Michael Niedermayer encoded:
> > On Mon, Aug 22, 2011 at 01:09:26AM +0200, Stefano Sabatini wrote:
> > > On date Sunday 2011-08-21 21:33:37 +0200, Michael Niedermayer encoded:
> > > > On Sun, Aug 21, 2011 at 07:38:35PM +0200, Michael Niedermayer wrote:
> > > > > On Sun, Aug 21, 2011 at 07:23:49PM +0200, Stefano Sabatini wrote:
> > > [...]
> > > > > > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> > > > > > index 03fc83a..4aae4db 100644
> > > > > > --- a/libavfilter/avfilter.h
> > > > > > +++ b/libavfilter/avfilter.h
> > > > > > @@ -102,7 +102,7 @@ typedef struct AVFilterBuffer {
> > > > > >  typedef struct AVFilterBufferRefAudioProps {
> > > > > >      int64_t channel_layout;     ///< channel layout of audio buffer
> > > > > >      int nb_samples;             ///< number of audio samples per channel
> > > > > > -    uint32_t sample_rate;       ///< audio buffer sample rate
> > > > > > +    int sample_rate;            ///< audio buffer sample rate
> > > > > >      int planar;                 ///< audio buffer - planar or packed
> > > > > >  } AVFilterBufferRefAudioProps;
> > > > > >
> > > > > 
> > > > > > @@ -607,7 +607,7 @@ struct AVFilterLink {
> > > > > >      AVRational sample_aspect_ratio; ///< agreed upon sample aspect ratio
> > > > > >      /* These parameters apply only to audio */
> > > > > >      int64_t channel_layout;     ///< channel layout of current buffer (see libavutil/audioconvert.h)
> > > > > > -    int64_t sample_rate;        ///< samples per second
> > > > > > +    int sample_rate;            ///< samples per second
> > > > > >      int planar;                 ///< agreed upon packing mode of audio buffers. true if planar.
> > > > > 
> > > > > this breaks ABI
> > > > 
> > > > Let me elaborate on this a bit
> > > > I know libavfilter isnt really stable ABI yet but ffmpeg
> > > > and ffplay use libavfilter and if one of their dependancies changes
> > > > ABI without soname/major ver bump then this leads to problems for
> > > > binaries of these tools
> > > 
> > > Yes I'm aware of this.
> > > 
> > > So what is the plan? Should we avoid ABI breaks, adding backward
> > > compatibility layers? Or can we simply break compatibility, relying on
> > > formal releases for users which need ABI stability?
> > 
> > If possible we should avoid ABI breaks relative to the last (0.8)
> > ABI
> > 
> > If that isnt possible we must bump soname & major version of
> > libavfilter
> 
> Options:
> 1. we keep pushing API/ABI breaks to lavfi and don't care about
>    backward compatibility issues, this was basically the policy
>    adopted until now.
> 
> 2. we bump when we need to bump (even if this would means different
>    bumps in a year), keeping a backward compatibility layer when
>    possible.
> 
> Others?
> 
> I'm fine with 2., that means much work for me but if this is going to
> simplify adoption/distro management, that's a reasonable price.

The bumping shouldnt add any work for you, its just ++ a value.
A compatibility layer does of course add work. But is not essential

Please also see for example
http://www.debian.org/doc/debian-policy/ch-sharedlibs.html
8.1 Run-time shared libraries
...." Every time the shared library ABI changes in a way that may break binaries linked against older versions of the shared library, the SONAME of the library and the corresponding name for the binary package containing the runtime shared library should change. Normally, this means the SONAME should change any time an interface is removed from the shared library or the signature of an interface (the number of parameters or the types of parameters that it takes, for example) is changed. This practice is vital to allowing clean upgrades from older versions of the package and clean transitions between the old ABI and new ABI without having to upgrade every affected package simultaneously. "



>
> In the case of this change, I can't see a simple way to handle it
> without bumping, can you?

Maybe i miss something but to me the change looks unneeded, if im
wrong, please elaborate what needs this change ?


Also if you want to do it, it can be done under

#if VERSION_MAJOR < ...
#else
#endif

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110823/858d8914/attachment.asc>