[FFmpeg-devel] [PATCH] deprecate SAMPLE_FMT_S24

[Michael Niedermayer]

On Thu, Aug 21, 2008 at 07:42:33PM +1000, 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.
> > > > > > > 
> > > 
> > > > bits_per_sample can NOT be used as SAMPLE_FMT_* bit count.
> > > > It has a different and docuented purpose, it describes the coded bitstream
> > > > and not the decoded samples, they match in bits only for lossless codecs.
> > > > 
> > > > example:
> > > > the a demuxer sets bits_per_sample to 4 and passes the data to a ADPCM
> > > > decoder which has SAMPLE_FMT_S16 output with 13 significant bits the
> > > > last 3 being always 0
> > > 
> > > Thanks for taking the time to explain this. Revised patch within.
> > > 
> > > The proposed field make sense only for SAMPLE_FMT_S32. I've kept the language
> > > generic, as it will have future utility when FFmpeg can manipulate 12/16-bit
> > > luma planes.
> > 
> > i think that maybe a
> > bits_per_coded_sample   // the number of bits in which each sample is coded
> > and
> > bits_per_raw_sample     // the number of bits in each PCM sample
> 
> Yes. Non-ambiguity is a desirable property. Done.
> 
> > would be better names, they are IMHO more clear
> > Iam not even sure that bits_per_sample is not sometimes used with the
> > expecttation that it is bits_per_raw_sample ...
> 
> An audit of bits_per_samples usage is on my todo list.
> 
> > 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.

patches ok

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

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20080825/eee9ed7f/attachment.pgp>