[FFmpeg-devel] ADPCM task (was Re: files in incoming)

Michael Niedermayer michaelni
Thu Jan 29 23:04:11 CET 2009


On Thu, Jan 29, 2009 at 08:40:53PM +0100, Stefan Gehrer wrote:
> Mike Melanson wrote:
> > Stefan Gehrer wrote:
> >> 2.) incoming/xbox-adpcm-wav/*
> >> Peter Ross posted a patch for supporting these in November.
> >> Since I just had my first experience with an ADPCM format
> >> I thought I could look at that too.
> > 
> > Working with ADPCM for the first time can be a very moving experience, I 
> > know.
> 
> I am not sure how to interpret this sentence. I currently
> file it under irony :)
> 
> > While you're waiting for Peter to re-upload those samples, perhaps 
> > you might wish to study adpcm_ea_r2. Check out these 2 files:
> > 
> > http://samples.mplayerhq.hu/game-formats/ea-mad/THX_logo.mad
> > http://samples.mplayerhq.hu/game-formats/ea-mpc/THX_logo.mpc
> > 
> > and probably other files from those 2 directories. Decode the audio using:
> > 
> >    ffmpeg -i <file> out.wav
> > 
> > The problem is that gcc 4.3.2 and gcc 4.4-svn decode the file in a very 
> > wrong way. And they each do it differently for each platform; 3 CPUs * 2 
> >   compilers => 6 different outputs:
> > 
> > (using 'ffmpeg -i <file> -f crc -'
> > 
> >    Linux / x86_64 / gcc 4.3.2
> > CRC=0x6c49e01a
> > 
> >    Linux / x86_64 / gcc svn
> > CRC=0x7dbbbe76
> > 
> >    Linux / x86_32 / gcc 4.3.2
> > CRC=0x78e518ec
> 
> This is the platform I happen to use and indeed playback was broken.
> The attached patch fixes it for me.
> The reason is that the + operator does not constitute a sequence point,
> thus the side-effects of the bytestream_get_x calls, i.e. incrementing
> src, can actually take place before reading src as the left operand.
> In case that explanation is correct, you have to thank Mans for
> teaching me this particular insight into C programming.
> 
> Independent of my patch, I think the pointer srcC should get some
> sanity checking at that point?
> 
> Stefan



> Index: libavcodec/adpcm.c
> ===================================================================
> --- libavcodec/adpcm.c	(revision 16828)
> +++ libavcodec/adpcm.c	(working copy)
> @@ -1300,9 +1300,10 @@
>          }
>  
>          for (channel=0; channel<avctx->channels; channel++) {
> -            srcC = src + (big_endian ? bytestream_get_be32(&src)
> -                                     : bytestream_get_le32(&src))
> -                       + (avctx->channels-channel-1) * 4;
> +            uint32_t offset = (big_endian ? bytestream_get_be32(&src)
> +                                          : bytestream_get_le32(&src));
> +                                     
> +            srcC = src + offset + (avctx->channels-channel-1) * 4;

the offset variable seems unneeded besides this looks ok

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20090129/050c069c/attachment.pgp>



More information about the ffmpeg-devel mailing list