[FFmpeg-devel] [PATCH] Handle all ADPCM codecs in av_get_bits_per_sample

[Michael Niedermayer]

On Tue, Jan 05, 2010 at 07:44:22PM -0500, Daniel Verkamp wrote:
> On Tue, Jan 5, 2010 at 6:16 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Jan 05, 2010 at 01:21:53PM -0500, Daniel Verkamp wrote:
> >> This patch partially fixes issue 1205 and eliminates special-case
> >> handling of ADPCM acodecs in libavformat/riff.c.
> >>
> >> Thanks,
> >> -- Daniel Verkamp
> >
> >> ?libavcodec/utils.c | ? ?5 +++++
> >> ?libavformat/riff.c | ? ?2 --
> >> ?2 files changed, 5 insertions(+), 2 deletions(-)
> >> 0747d0e35a2b9a34ffa1874465735d58c9684d1e ?0001-Handle-all-ADPCM-codecs-in-av_get_bits_per_sample.patch
> >> From 02c07cb8fc4fe40e66764882db4aef42f64ad6da Mon Sep 17 00:00:00 2001
> >> From: Daniel Verkamp <daniel at drv.nu>
> >> Date: Tue, 5 Jan 2010 13:19:16 -0500
> >> Subject: [PATCH] Handle all ADPCM codecs in av_get_bits_per_sample
> >>
> >> Some ADPCM codecs were special-cased in riff.c;
> >> move them to generic code.
> >> ---
> >> ?libavcodec/utils.c | ? ?5 +++++
> >> ?libavformat/riff.c | ? ?2 --
> >> ?2 files changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >> index 99c97c1..6e914b9 100644
> >> --- a/libavcodec/utils.c
> >> +++ b/libavcodec/utils.c
> >> @@ -963,6 +963,11 @@ int av_get_bits_per_sample(enum CodecID codec_id){
> >> ? ? ? ? ?return 3;
> >> ? ? ?case CODEC_ID_ADPCM_SBPRO_4:
> >> ? ? ?case CODEC_ID_ADPCM_CT:
> >> + ? ?case CODEC_ID_ADPCM_IMA_WAV:
> >> + ? ?case CODEC_ID_ADPCM_MS:
> >> + ? ?case CODEC_ID_ADPCM_G726:
> >> + ? ?case CODEC_ID_ADPCM_YAMAHA:
> >> + ? ?case CODEC_ID_ADPCM_SWF:
> >> ? ? ? ? ?return 4;
> >
> > some of these are not fixed 4 bit
> >
> 
> I am unsure what exactly av_get_bits_per_sample() should return for
> things that aren't simple PCM; any clarification would be appreciated.
> 
> Attempting to decide which one of these are not fixed 4 bit:
> 
> CODEC_ID_ADPCM_IMA_WAV: fixed 4-bit samples, but packed 8 samples at a
> time per channel for stereo (does this matter for
> av_get_bits_per_sample()?) (adpcm.c:994-1003)
> 
> CODEC_ID_ADPCM_MS: fixed 4-bit samples with some header data per block
> (adpcm.c:1059-1063)
> 
> CODEC_ID_ADPCM_G726: 2-, 3-, 4-, or 5-bit samples (no way to tell by
> codec id?) (g726.c:327)
> 
> CODEC_ID_ADPCM_YAMAHA: fixed 4-bit samples with no per-block header
> (adpcm.c:1551-1564)
> 
> CODEC_ID_ADPCM_SWF: 2- or 4-bit samples (no way to tell by codec id?)
> 
> If there is a header coded before normal samples, does/should that
> affect the av_get_bits_per_sample() result? (For example, the
> ADPCM_SBPRO variants have the initial sample of a block coded as a
> full byte but still return 2/3/4.)

Well i dont see how the header could be considered so i guess this isnt
that much a case where we have a choice ...


> 
> Assuming existing semantics are correct, ok to move fixed 4-bit codecs
> CODEC_ID_ADPCM_IMA_WAV, CODEC_ID_ADPCM_MS, and CODEC_ID_ADPCM_YAMAHA
> to av_get_bits_per_sample()?

yes


> 
> Additionally there is the issue of the existing cases in riff.c; all
> of the above are already indicated as 4 bits per sample (including
> G726, which seems to support several sample sizes); is this correct or
> an oversight?

This is hard to awnser with riff.c, only thing sure is that any change
to the values there must be tested. Simply assuming tat what is logic 
is correct is not guranteed to work. Unless you have some official files
that use such different value.


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

Democracy is the form of government in which you can choose your dictator
-------------- 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/20100106/610e99a6/attachment.pgp>