[FFmpeg-devel] [PATCH] mp3on4 heavy cleanup

[Reimar Döffinger]

On Fri, Apr 25, 2008 at 04:56:58PM +0200, Baptiste Coudurier wrote:
> Index: libavcodec/mpegaudiodec.c
> ===================================================================
> --- libavcodec/mpegaudiodec.c	(revision 12960)
> +++ libavcodec/mpegaudiodec.c	(working copy)
> @@ -2581,13 +2581,10 @@
>  
>      for (fr = 0; fr < s->frames; fr++) {
>          start = start2;
> -        fsize = (start[0] << 4) | (start[1] >> 4);
> +        fsize = AV_RB16(start[0]) >> 4;

Huh? I think that should be "start" instead of "start[0]"?

> +        fsize = FFMIN(fsize, FFMIN(len, MPA_MAX_CODED_FRAME_SIZE));

Create a FFMIN3 just like the FFMAX3 IMO.

>  /* Next 3 arrays are indexed by channel config number (passed via codecdata) */
> -static int mp3Frames[16] = {0,1,1,2,3,3,4,5,2};   /* number of mp3 decoder instances */
> -static int mp3Channels[16] = {0,1,2,3,4,5,6,8,4}; /* total output channels */
> +static const uint8_t mp3_frames[8] = {0,1,1,2,3,3,4,5}; /* number of mp3 decoder instances */
>  /* offsets into output buffer, assume output order is FL FR BL BR C LFE */
> -static int chan_offset[9][5] = {
> +static const uint8_t chan_offset[9][5] = {

That should probably be applied separately. IMO you do not need to send
a patch for such things.

> @@ -2475,11 +2476,13 @@
>  #endif /* CONFIG_MP3ADU_DECODER */
>  
>  #ifdef CONFIG_MP3ON4_DECODER
> +
> +#include "mpeg4audio.h"
> +
>  /* Next 3 arrays are indexed by channel config number (passed via codecdata) */
> -static int mp3Frames[16] = {0,1,1,2,3,3,4,5,2};   /* number of mp3 decoder instances */
> -static int mp3Channels[16] = {0,1,2,3,4,5,6,8,4}; /* total output channels */
> +static const uint8_t mp3_frames[8] = {0,1,1,2,3,3,4,5}; /* number of mp3 decoder instances */
>  /* offsets into output buffer, assume output order is FL FR BL BR C LFE */
> -static int chan_offset[9][5] = {
> +static const uint8_t chan_offset[9][5] = {
>      {0},
>      {0},            // C
>      {0},            // FLR
> @@ -2495,6 +2498,7 @@

Did you include that patch twice or is all that code duplicated??

Greetings,
Reimar D?ffinger