[FFmpeg-devel] [RFC] wrong MS ADPC decoding

[Michael Niedermayer]

On Sat, Jul 12, 2008 at 11:53:18AM +0200, Reimar D?ffinger wrote:
> Hello,
> by comparing with MPlayer decoder and an listen test with
> http://samples.mplayerhq.hu/A-codecs/msadpcm-stereo/dance-b5th.asf I
> came to the conclusion that the FFmpeg decoder decodes incorrectly,
> the directly-stored samples should be swapped, as with attached patch.
> Could you please verify?
> Looking at the PSNR of the new encoder regression tests there is still
> something wrong, and the last ca. 12 kB of the decoded audio still
> differs between MPlayer and FFmpeg decoders...
> 
> Greetings,
> Reimar D?ffinger

> diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> index a9a8f74..91fe9a0 100644
> --- a/libavcodec/adpcm.c
> +++ b/libavcodec/adpcm.c
> @@ -605,14 +605,14 @@ static int adpcm_encode_frame(AVCodecContext *avctx,
>          }
>          for(i=0; i<avctx->channels; i++){
>              c->status[i].sample1= *samples++;
> -
> -            bytestream_put_le16(&dst, c->status[i].sample1);
>          }
>          for(i=0; i<avctx->channels; i++){
>              c->status[i].sample2= *samples++;
>  
>              bytestream_put_le16(&dst, c->status[i].sample2);
>          }
> +        for(i=0; i<avctx->channels; i++)
> +            bytestream_put_le16(&dst, c->status[i].sample1);

This looks wrong



>  
>          if(avctx->trellis > 0) {
>              int n = avctx->block_align - 7*avctx->channels;
> @@ -1050,10 +1050,10 @@ static int adpcm_decode_frame(AVCodecContext *avctx,
>          if (st) c->status[1].sample2 = ((*src & 0xFF) | ((src[1] << 8) & 0xFF00));
>          if (st) src+=2;
>  
> -        *samples++ = c->status[0].sample1;
> -        if (st) *samples++ = c->status[1].sample1;
>          *samples++ = c->status[0].sample2;
>          if (st) *samples++ = c->status[1].sample2;
> +        *samples++ = c->status[0].sample1;
> +        if (st) *samples++ = c->status[1].sample1;
>          for(;n>0;n--) {
>              *samples++ = adpcm_ms_expand_nibble(&c->status[0 ], src[0] >> 4  );
>              *samples++ = adpcm_ms_expand_nibble(&c->status[st], src[0] & 0x0F);

this looks correct

btw, just in case you dont have it, the MS-ADPCM algo is descibed in 
RIFFNEW.pdf (google will find it i suspect)

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20080713/1cdae256/attachment.pgp>