[FFmpeg-devel] [PATCH] use bytestream functions more in adpcm.c

[Ramiro Polla]

Hello,

  wrote:
> Hello,
> seems there is some ancient and horribly obfuscated code in there.
> Attached patch replaces most (all?) of them.
> Regression tests succeed, but I only manually tested on MS ADPCM sample.
> Btw. is that file indeed unmaintained?

[...]

> Index: libavcodec/adpcm.c
> ===================================================================
> --- libavcodec/adpcm.c	(revision 14175)
> +++ libavcodec/adpcm.c	(working copy)
[...]
> @@ -1064,14 +1057,14 @@
>          if (avctx->block_align != 0 && buf_size > avctx->block_align)
>              buf_size = avctx->block_align;
>  
> -        c->status[0].predictor = (int16_t)(src[0] | (src[1] << 8));
> -        c->status[0].step_index = src[2];
> -        src += 4;
> +        c->status[0].predictor = (int16_t)bytestream_get_le16(&src);
> +        c->status[0].step_index = *src++;
> +        src++;
>          *samples++ = c->status[0].predictor;
>          if (st) {
> -            c->status[1].predictor = (int16_t)(src[0] | (src[1] << 8));
> -            c->status[1].step_index = src[2];
> -            src += 4;
> +            c->status[1].predictor = (int16_t)bytestream_get_le16(&src);
> +            c->status[1].step_index = *src++;
> +            src++;

I think these look better if vertically aligned.

> @@ -1196,14 +1189,10 @@
>              break;
>          }
>          src += 4;
> -        current_left_sample = (int16_t)AV_RL16(src);
> -        src += 2;
> -        previous_left_sample = (int16_t)AV_RL16(src);
> -        src += 2;
> -        current_right_sample = (int16_t)AV_RL16(src);
> -        src += 2;
> -        previous_right_sample = (int16_t)AV_RL16(src);
> -        src += 2;
> +        current_left_sample = (int16_t)bytestream_get_le16(&src);
> +        previous_left_sample = (int16_t)bytestream_get_le16(&src);
> +        current_right_sample = (int16_t)bytestream_get_le16(&src);
> +        previous_right_sample = (int16_t)bytestream_get_le16(&src);
>  
>          for (count1 = 0; count1 < samples_in_chunk/28;count1++) {
>              coeff1l = ea_adpcm_table[ *src >> 4       ];

Same thing here and maybe in some other places.

I looked at this file yesterday and thought about these same changes 
(but was too lazy to actually do them).

The file has lots of code that would be more readable if vertically 
aligned. I also thought about moving the cases inside the huge switches 
in decode/encode frame into their own functions, so that they can easily 
be #ifdef'd out undef CONFIG_ADPCM_BLAH.

Ramiro Polla