[FFmpeg-devel] [PATCH] EA ADPCM R1, R2 and R3

[Michael Niedermayer]

Hi

On Sat, Oct 20, 2007 at 03:42:35PM +0200, Aurelien Jacobs wrote:
> Hi,
> 
> Attached patch adds support for EA ADPCM R1, R2 and R3.
> It's based on the patch from Peter Ross.
> I intend to apply it soon.
> 
> Aurel

[...]
> @@ -637,9 +638,12 @@
>  {
>      ADPCMContext *c = avctx->priv_data;
>  
> -    if(avctx->channels > 2U){
> +    if ((avctx->channels > 2
> +         && avctx->codec->id != CODEC_ID_ADPCM_EA_R1
> +         && avctx->codec->id != CODEC_ID_ADPCM_EA_R2
> +         && avctx->codec->id != CODEC_ID_ADPCM_EA_R3)
> +        || avctx->channels > 6)
>          return -1;
> -    }

this is wrong, channels is signed so it must be 2U not 2

also this is not the most readable way to check this especially if more
codecs get added ...


>  
>      c->channel = 0;
>      c->status[0].predictor = c->status[1].predictor = 0;
> @@ -1182,6 +1186,78 @@
>              }
>          }
>          break;
> +    case CODEC_ID_ADPCM_EA_R1:
> +    case CODEC_ID_ADPCM_EA_R2:
> +    case CODEC_ID_ADPCM_EA_R3: {
> +        /* channel numbering
> +           2chan: 0=fl, 1=fr
> +           4chan: 0=fl, 1=rl, 2=fr, 3=rr
> +           6chan: 0=fl, 1=c,  2=fr, 3=rl,  4=rr, 5=sub */
> +        const int big_endian = avctx->codec->id == CODEC_ID_ADPCM_EA_R3;
> +        unsigned int channel;
> +        uint16_t *samplesC;
> +        uint8_t *srcC;
> +
> +        samples_in_chunk = (big_endian ? bytestream_get_be32(&src)
> +                                       : bytestream_get_le32(&src)) / 28;
> +
> +        for (channel=0; channel<avctx->channels; channel++) {
> +            srcC = src + (big_endian ? bytestream_get_be32(&src)
> +                                     : bytestream_get_le32(&src))
> +                       + (avctx->channels-channel-1) * 4;
> +            samplesC = samples + channel;
> +
> +            if (avctx->codec->id == CODEC_ID_ADPCM_EA_R1) {
> +                current_left_sample  = (int16_t)bytestream_get_le16(&srcC);
> +                previous_left_sample = (int16_t)bytestream_get_le16(&srcC);
> +            } else {
> +                current_left_sample  = c->status[channel].predictor;
> +                previous_left_sample = c->status[channel].prev_sample;
> +            }
> +
> +            for (count1=0; count1<samples_in_chunk; count1++) {
> +                if (*srcC == 0xEE) {  /* only seen in R2 and R3 */
> +                    srcC++;
> +                    current_left_sample  = (int16_t)bytestream_get_be16(&srcC);
> +                    previous_left_sample = (int16_t)bytestream_get_be16(&srcC);
> +
> +                    for (count2=0; count2<28; count2++) {
> +                        *samplesC = (int16_t)bytestream_get_be16(&srcC);
> +                        samplesC += avctx->channels;
> +                    }

completely missing checks for array bounds, this should be exploitable


> +                } else {
> +                    coeff1l = ea_adpcm_table[ (*srcC>>4) & 0x0F     ];
> +                    coeff2l = ea_adpcm_table[((*srcC>>4) & 0x0F) + 4];
> +                    shift_left = (*srcC++ & 0x0F) + 8;
> +
> +                    for (count2=0; count2<28; count2++) {
> +                        if (count2 & 1)
> +                            next_left_sample = ((*srcC++&0x0F)<<28)>>shift_left;
> +                        else
> +                            next_left_sample = ((*srcC  &0xF0)<<24)>>shift_left;

this code doesnt seem to be executed with just the left channel so why
is it called left?


> +
> +                        next_left_sample += (current_left_sample  * coeff1l) +
> +                                            (previous_left_sample * coeff2l);
> +                        next_left_sample = av_clip_int16(next_left_sample >> 8);
> +
> +                        previous_left_sample = current_left_sample;
> +                        current_left_sample  = next_left_sample;

> +                        *samplesC = (int16_t)current_left_sample;

senseless cast

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20071020/78ea3f10/attachment.pgp>