[FFmpeg-devel] [PATCH] deprecate SAMPLE_FMT_S24

[Peter Ross]

On Thu, Aug 21, 2008 at 04:43:48PM +0200, Aurelien Jacobs wrote:
> Peter Ross wrote:
> 
> > On Tue, Aug 19, 2008 at 03:31:52PM +0200, Michael Niedermayer wrote:
> > > On Tue, Aug 19, 2008 at 10:29:32PM +1000, Peter Ross wrote:
> > > > On Sun, Aug 17, 2008 at 05:10:39PM +0200, Michael Niedermayer wrote:
> > > > > On Sun, Aug 17, 2008 at 04:37:49PM +0200, Aurelien Jacobs wrote:
> > > > > > Michael Niedermayer wrote:
> > > > > > 
> > > > > > > On Sun, Aug 17, 2008 at 03:32:30PM +0200, Aurelien Jacobs wrote:
> > > > > > > > Peter Ross wrote:
> > > > > > > > 
> > > > > > > > > This patch flags SAMPLE_FMT_S24 as deprecated.

> > > I wonder if anyone would complain if we just renamed bits_per_sample to
> > > bits_per_coded_sample ? It doesnt break ABI, just API of apps accessing it
> > > without AVOptions and apps really should be using AVOptions
> > 
> > IMHO it should be guarded with an ifdef until the next major version bump.
> > Patch enclosed.
> 
> I somewhat disagree here. Changing the field name don't breaks ABI, so no
> need to make it conditional to version bump.
> Instead you could just add a compatibility #define. See below patch
> (hand edited, untested...)

> -     int bits_per_sample;
> +#if LIBAVCODEC_VERSION_MAJOR < 52
> +#define bits_per_sample bits_per_coded_sample
> +#endif
> +     int bits_per_coded_sample;

My concern is that this would break many user application that uses the
'bit_per_sample' literal in its code. e.g. the following program would break.

#include <user.h>    // defines struct { int bits_per_sample; } foo;
#include <libavcodec/avcodec.h>
int main() {
   struct foo bar = { .bits_per_sample = 1 };
}

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- 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/20080822/20ff6612/attachment.pgp>