[FFmpeg-devel] [PATCH] adpcm-ima-wav header and codec

[Michael Niedermayer]

On Mon, Oct 15, 2007 at 07:55:29PM +0400, Timofei wrote:
> Hi.
>
> I found a few bugs in the adpcm-ima codec.
> This format is used in smv video by sigmatel-based mp3 players.
>
> 1) The field 'bytes-per-second' in WAV header is set incorrectly.
> This breaks seeking on hardware player.
> I found the bitrate value used in calculations comes from commandline and 
> does not reflect reality.
> The adpcm-bitrate.patch puts correct bitrate in avctx.
> This fixes wav header, but the ffmpeg program still prints wrong bitrate
> during encoding.
>
> 2) adpcm-codec.patch
> In adpcm_encode_frame() predictor for right channel gets initialized by 2nd 
> sample of the left channel instead of 1st sample of the right channel.
>
> The adpcm_decode_frame() loses first sample in block.
> This sample initializes predictor and then gets lost instead of being 
> copied to output.
>
> 3) adpcm-index_step.patch - initialization of index_step added.
> This patch depends on adpcm-codec.patch
>
> 4) adpcm-guard.patch - not a bug. Just removed unnecessary guarding &=0x0f 
> around adpcm_ima_compress_sample().
>
> Regards.
> 	Tim.

> --- ffmpeg-svn/libavcodec/adpcm.c	2007-10-12 12:21:16.000000000 +0400
> +++ ffmpeg/libavcodec/adpcm.c	2007-10-12 19:01:53.000000000 +0400
> @@ -166,6 +166,7 @@
>                                                               /* and we have 4 bytes per channel overhead */
>          avctx->block_align = BLKSIZE;
>          /* seems frame_size isn't taken into account... have to buffer the samples :-( */
> +	avctx->bit_rate = avctx->sample_rate * avctx->block_align * 8 / avctx->frame_size;

tabs are forbidden in ffmpeg svn
and
     /**
     * the average bitrate
     * - encoding: Set by user; unused for constant quantizer encoding.
     * - decoding: Set by libavcodec. 0 or some bitrate if this info is available in the stream.
     */
    int bit_rate;

that is its set by the user not by libavcodec acording to API



>          break;
>      case CODEC_ID_ADPCM_MS:
>          avctx->frame_size = (BLKSIZE - 7 * avctx->channels) * 2 / avctx->channels + 2; /* each 16 bits sample gives one nibble */

> --- ffmpeg-svn/libavcodec/adpcm.c	2007-10-12 12:21:16.000000000 +0400
> +++ ffmpeg/libavcodec/adpcm.c	2007-10-12 19:01:53.000000000 +0400
> @@ -448,19 +449,17 @@
>          break;
>      case CODEC_ID_ADPCM_IMA_WAV:
>          n = avctx->frame_size / 8;
> -            c->status[0].prev_sample = (signed short)samples[0]; /* XXX */
> +            c->status[0].prev_sample = *samples++;
>  /*            c->status[0].step_index = 0; *//* XXX: not sure how to init the state machine */
>              bytestream_put_le16(&dst, c->status[0].prev_sample);
>              *dst++ = (unsigned char)c->status[0].step_index;
>              *dst++ = 0; /* unknown */
> -            samples++;

this change is purely cosmetic and belongs in a seperate patch


[...]

> --- ffmpeg-svn/libavcodec/adpcm.c	2007-10-12 12:21:16.000000000 +0400
> +++ ffmpeg/libavcodec/adpcm.c	2007-10-12 19:01:53.000000000 +0400
> @@ -483,17 +483,17 @@

looks ok


> --- ffmpeg-svn/libavcodec/adpcm.c	2007-10-12 12:21:16.000000000 +0400
> +++ ffmpeg/libavcodec/adpcm.c	2007-10-12 19:01:53.000000000 +0400
> @@ -220,6 +220,16 @@ static inline unsigned char adpcm_ima_co
>      return nibble;
>  }
>  
> +static void inline adpcm_ima_init_step_index(ADPCMChannelStatus *c, short next_sample)

i dont think that this should be inline, or what is the speed gain from that?

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20071015/9af22a30/attachment.pgp>