[FFmpeg-devel] [PATCH] AAC decoder
Michael Niedermayer
michaelni
Wed Apr 2 21:40:25 CEST 2008
On Tue, Apr 01, 2008 at 04:56:48PM +0200, Andreas ?man wrote:
> Andreas ?man wrote:
>> Hi
>> Here is the AAC decodoer from the soc repo.
>
> .. and here is an updated version with mpeg4audio.[ch] stuff.
[...]
> +static const uint16_t swb_offset_1024_96[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56,
> + 64, 72, 80, 88, 96, 108, 120, 132, 144, 156, 172, 188, 212, 240,
> + 276, 320, 384, 448, 512, 576, 640, 704, 768, 832, 896, 960, 1024
> +};
> +
> +static const uint16_t swb_offset_128_96[] = {
> + 0, 4, 8, 12, 16, 20, 24, 32, 40, 48, 64, 92, 128
> +};
> +
> +static const uint16_t swb_offset_1024_64[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56,
> + 64, 72, 80, 88, 100, 112, 124, 140, 156, 172, 192, 216, 240, 268,
> + 304, 344, 384, 424, 464, 504, 544, 584, 624, 664, 704, 744, 784, 824,
> + 864, 904, 944, 984, 1024
> +};
> +
> +static const uint16_t swb_offset_1024_48[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 48, 56, 64, 72,
> + 80, 88, 96, 108, 120, 132, 144, 160, 176, 196, 216, 240, 264, 292,
> + 320, 352, 384, 416, 448, 480, 512, 544, 576, 608, 640, 672, 704, 736,
> + 768, 800, 832, 864, 896, 928, 1024
> +};
> +
> +static const uint16_t swb_offset_128_48[] = {
> + 0, 4, 8, 12, 16, 20, 28, 36, 44, 56, 68, 80, 96, 112, 128
> +};
> +
> +static const uint16_t swb_offset_1024_32[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 48, 56, 64, 72,
> + 80, 88, 96, 108, 120, 132, 144, 160, 176, 196, 216, 240, 264, 292,
> + 320, 352, 384, 416, 448, 480, 512, 544, 576, 608, 640, 672, 704, 736,
> + 768, 800, 832, 864, 896, 928, 960, 992, 1024
> +};
> +
> +
> +static const uint16_t swb_offset_1024_24[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 52, 60, 68,
> + 76, 84, 92, 100, 108, 116, 124, 136, 148, 160, 172, 188, 204, 220,
> + 240, 260, 284, 308, 336, 364, 396, 432, 468, 508, 552, 600, 652, 704,
> + 768, 832, 896, 960, 1024
> +};
> +
> +static const uint16_t swb_offset_128_24[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 36, 44, 52, 64, 76, 92, 108, 128
> +};
> +
> +static const uint16_t swb_offset_1024_16[] = {
> + 0, 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 88, 100, 112, 124,
> + 136, 148, 160, 172, 184, 196, 212, 228, 244, 260, 280, 300, 320, 344,
> + 368, 396, 424, 456, 492, 532, 572, 616, 664, 716, 772, 832, 896, 960, 1024
> +};
> +
> +static const uint16_t swb_offset_128_16[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 32, 40, 48, 60, 72, 88, 108, 128
> +};
> +
> +static const uint16_t swb_offset_1024_8[] = {
> + 0, 12, 24, 36, 48, 60, 72, 84, 96, 108, 120, 132, 144, 156, 172,
> + 188, 204, 220, 236, 252, 268, 288, 308, 328, 348, 372, 396, 420, 448,
> + 476, 508, 544, 580, 620, 664, 712, 764, 820, 880, 944, 1024
> +};
> +
> +static const uint16_t swb_offset_128_8[] = {
> + 0, 4, 8, 12, 16, 20, 24, 28, 36, 44, 52, 60, 72, 88, 108, 128
> +};
> +
> +static const uint16_t *swb_offset_1024[] = {
> + swb_offset_1024_96, swb_offset_1024_96, swb_offset_1024_64,
> + swb_offset_1024_48, swb_offset_1024_48, swb_offset_1024_32,
> + swb_offset_1024_24, swb_offset_1024_24, swb_offset_1024_16,
> + swb_offset_1024_16, swb_offset_1024_16, swb_offset_1024_8
> +};
All these tables look like they contain multiples of 4, so if they were
divided by 4 they would almost fit in uint8_t (i assume both the 0 and
1024 elements are really accessed?)
But then maybe this is stupid ...
[...]
> +/**
> + * Special codebooks
> + */
> +#define ZERO_HCB 0
> +#define FIRST_PAIR_HCB 5
> +#define ESC_HCB 11
> +#define NOISE_HCB 13
> +#define INTENSITY_HCB2 14
> +#define INTENSITY_HCB 15
> +#define ESC_FLAG 16
This doxy comment is associated just with the first entry
also this can be vertically aligned
> +
> +/**
> + * Channel types
> + */
> +#define AAC_CHANNEL_FRONT 1
> +#define AAC_CHANNEL_SIDE 2
> +#define AAC_CHANNEL_BACK 3
> +#define AAC_CHANNEL_LFE 4
> +#define AAC_CHANNEL_CC 5
> +
looks like it should be an enum
[...]
> +/**
> + * Individual Channel Stream
> + */
> +typedef struct {
> + int intensity_present;
> + int max_sfb;
> + int window_sequence;
> + int window_shape; ///< If set, use Kaiser-Bessel window, otherwise use a sinus window
> + int window_shape_prev;
> + int num_window_groups;
> + uint8_t grouping;
> + uint8_t group_len[8];
> + ltp_struct ltp;
> + ltp_struct ltp2;
ltp[2]; ?
[...]
> +/**
> + * Per-element gain control for SSR
> + */
> +typedef struct {
> + int max_band;
> + int adjust_num[4][8];
> + int alev[4][8][8];
> + int aloc[4][8][8];
> + float buf[4][24];
> +} ssr_struct;
> +
> +/**
> + * Coupling parameters
> + */
> +typedef struct {
> + int ind_sw; ///< Set if independant coupling (i.e. after IMDCT)
> + int domain; ///< Controls if coupling is performed before (0) or after (1) the TNS decoding of the target channels
> + int num_coupled; ///< Number of target elements
> + int is_cpe[9]; ///< Set if target is an CPE (otherwise it's an SCE)
> + int tag_select[9]; ///< Element tag index
> + int l[9]; ///< Apply gain to left channel of an CPE
> + int r[9]; ///< Apply gain to right channel of an CPE
> + float gain[18][8][64];
> +} coupling_struct;
> +
> +
> +/**
> + * Single Channel Element
> + * Used for both SCE and LFE elements
> + */
> +typedef struct {
> + float mixing_gain; /**< Channel gain (not used by AAC bitstream).
> + * Note that this is applied before joint stereo decoding.
> + * Thus, when used inside CPE elements, both channels must have equal gain.
> + */
> + ics_struct ics;
> + tns_struct tns;
> + int cb[8][64]; ///< Codebooks
> + float sf[8][64]; ///< Scalefactors
> + DECLARE_ALIGNED_16(float, coeffs[1024]); ///< Coefficients for IMDCT
> + DECLARE_ALIGNED_16(float, saved[1024]); ///< Overlap
> + DECLARE_ALIGNED_16(float, ret[1024]); ///< PCM output
> + int16_t *ltp_state;
> + ssr_struct *ssr;
> +} sce_struct;
> +
> +/**
> + * Channel Pair Element
> + */
> +typedef struct {
> + int common_window; ///< Set if channels share a common 'ics_struct' in bitstream
> + ms_struct ms;
> + sce_struct ch[2];
> +} cpe_struct;
> +
> +/**
> + * Channel Coupling
> + */
> +typedef struct {
> + coupling_struct coup;
> + sce_struct ch;
> +} cc_struct;
I think, though i may be wrong, that all sce_struct should be in a flat array
for easy access. And not spread over dozens of other structs. That of course
only if it does lead to simpler code, but i see several places where code
is duplicated for each of these wraper structs.
[...]
> +// aux
> +/**
> + * Generate a sine Window.
> + */
> +static void sine_window_init(float *window, int n) {
> + const float alpha = M_PI / n;
> + int i;
> + for(i = 0; i < n/2; i++)
> + window[i] = sin((i + 0.5) * alpha);
> +}
> +
> +#ifdef AAC_SSR
> +static void ssr_context_init(ssr_context * ctx) {
> + int b, i;
> + for (b = 0; b < 2; b++) {
> + for (i = 0; i < 4; i++) {
> + // 2
> + ctx->q[b][i] = cos((2*i+1)*(2*b+1-4)*M_PI/16);
> + ctx->q[b+2][i] = cos((2*i+1)*(2*(b+4)+1-4)*M_PI/16);
> + }
> + }
vertial align
ctx->q[b ][i] = cos((2*i+1)*(2* b +1-4)*M_PI/16);
ctx->q[b+2][i] = cos((2*i+1)*(2*(b+4)+1-4)*M_PI/16);
ive not looked at the code using this but b+1 isnt set?
> + for (b = 0; b < 4; b++) {
> + for (i = 0; i < 12; i++) {
> + float sgn = 1 - 2 * (i&1); //4
> + if (i < 6) {
> + ctx->t0[b][i] = sgn * ssr_q_table[8*i + b];
> + ctx->t1[b][i] = sgn * ssr_q_table[8*i + b + 4];
> + } else {
> + ctx->t0[b][i] = sgn * ssr_q_table[95 - (8*i + b)];
> + ctx->t1[b][i] = sgn * ssr_q_table[95 - (8*i + b + 4)];
> + }
> + }
> + }
> +}
This stuff also looks like it should be in static (const) tables
[...]
> +/**
> + * Configure output channel order and optional mixing based on the current
> + * program config element and user requested channels.
> + *
> + * \param nwepcs New program config struct. We only do somthing if it differs from the current one.
> + */
> +static int output_configure(AACContext *ac, program_config_struct *newpcs) {
> + AVCodecContext *avctx = ac->avccontext;
> + program_config_struct * pcs = &ac->pcs;
> + int i, channels = 0, ch;
> + float a, b;
> + cpe_struct *front = NULL, *back = NULL;
> + sce_struct *center = NULL;
> +
> + static const float mixdowncoeff[4] = {
> + /* Matrix mixdown coefficient, Table 4.70 */
> + 1. / M_SQRT2,
> + 1. / 2.,
> + 1. / (2 * M_SQRT2),
> + 0
> + };
> +
> + if(!memcmp(&ac->pcs, newpcs, sizeof(program_config_struct)))
> + return 0; /* no change */
> +
> + memcpy(pcs, newpcs, sizeof(program_config_struct));
*pcs= *newpcs;
> +
> + /* Allocate or free elements depending on if they are in the
> + current program config struct */
> +
> + for(i = 0; i < MAX_TAGID; i++) {
> + channels += !!pcs->sce_type[i] + !!pcs->cpe_type[i] * 2 + !!pcs->lfe_type[i];
> +
> + if(pcs->sce_type[i]) {
> + if(!ac->che_sce[i]) ac->che_sce[i] = av_mallocz(sizeof(sce_struct));
> + } else
> + sce_freep(&ac->che_sce[i]);
> +
> + if(pcs->cpe_type[i]) {
> + if(!ac->che_cpe[i]) ac->che_cpe[i] = av_mallocz(sizeof(cpe_struct));
> + } else
> + cpe_freep(&ac->che_cpe[i]);
> +
> + if(pcs->lfe_type[i]) {
> + if(!ac->che_lfe[i]) ac->che_lfe[i] = av_mallocz(sizeof(sce_struct));
> + } else
> + sce_freep(&ac->che_lfe[i]);
> +
> + if(pcs->cc_type[i]) {
> + if(!ac->che_cc[i]) ac->che_cc[i] = av_mallocz(sizeof(cc_struct));
> + } else
> + cc_freep(&ac->che_cc[i]);
This code duplication can be avoided.
> + }
> +
> + /* Setup default 1:1 output mapping.
> + *
> + * For a 5.1 stream the output order will be:
> + * [ Front Left ] [ Front Right ] [ Center ] [ LFE ] [ Surround Left ] [ Surround Right ]
> + *
> + * While at it: locate front, center and back for matrix mixdown further down
> + */
> +
> + ch = 0;
> + for(i = 0; i < MAX_TAGID; i++) {
> +
> + if(pcs->cpe_type[i]) {
> + ac->output_data[ch++] = ac->che_cpe[i]->ch[0].ret;
> + ac->output_data[ch++] = ac->che_cpe[i]->ch[1].ret;
> +
> + ac->che_cpe[i]->ch[0].mixing_gain = 1.0f;
> + ac->che_cpe[i]->ch[1].mixing_gain = 1.0f;
> +
> + if(!front && pcs->cpe_type[i] == AAC_CHANNEL_FRONT)
> + front = ac->che_cpe[i];
> +
> + if(!back && pcs->cpe_type[i] == AAC_CHANNEL_BACK)
> + back = ac->che_cpe[i];
> + }
> +
> + if(pcs->sce_type[i]) {
> + ac->output_data[ch++] = ac->che_sce[i]->ret;
> + ac->che_sce[i]->mixing_gain = 1.0f;
> +
> + if(!center && pcs->sce_type[i] == AAC_CHANNEL_FRONT)
> + center = ac->che_sce[i];
> + }
> + if(ac->che_lfe[i]) {
> + ac->output_data[ch++] = ac->che_lfe[i]->ret;
> + ac->che_lfe[i]->mixing_gain = 1.0f;
> + }
This also looks like some basic block being duplicated a few times.
> + }
> + assert(ch == channels);
> +
> + ac->mm_front = ac->mm_back = NULL;
> + ac->mm_center = NULL;
> +
> + /* Check for matrix mixdown to mono or stereo */
> +
> + if(avctx->request_channels != 0 && avctx->request_channels <= 2 &&
!= 0 is superflous
[...]
> + if(avctx->request_channels == 2) {
> + b = 1. / (1. + (1. / M_SQRT2) + a * (pcs->pseudo_surround ? 2. : 1.));
> +
> + front->ch[0].mixing_gain = b;
> + front->ch[1].mixing_gain = b;
> + center->mixing_gain = b / M_SQRT2;
> + back->ch[0].mixing_gain = b * a;
> + back->ch[1].mixing_gain = b * a;
> + } else {
> + b = 1. / (3. + 2. * a);
> + front->ch[0].mixing_gain = b;
> + front->ch[1].mixing_gain = b;
> + center->mixing_gain = b;
> + back->ch[0].mixing_gain = b * a;
> + back->ch[1].mixing_gain = b * a;
> + }
some code can be factored out of that if/else
[...]
> +/**
> + * Parse program config element
> + * reference: Table 4.2
> + */
> +static int program_config_element(AACContext * ac, GetBitContext * gb) {
> + program_config_struct pcs;
> + int i, num_front, num_side, num_back, num_lfe, num_assoc_data, num_cc;
> +
> + memset(&pcs, 0, sizeof(program_config_struct));
sizeof(pcs)
[...]
> + if (get_bits1(gb)) {
> + pcs.mixdown_coeff_index = get_bits(gb, 2);
> + pcs.pseudo_surround = get_bits1(gb);
> + } else {
> + pcs.mixdown_coeff_index = 0;
> + pcs.pseudo_surround = 0;
> + }
the else is redundant due to the memset
[...]
> + // not a real audio channel
> + for (i = 0; i < num_assoc_data; i++)
> + skip_bits(gb, 4);
skip_bits_long()
[...]
> +/**
> + * Set up program_config_struct, but based on a default channel configuration
> + * as specified in Table 1.17
> + */
> +static int program_config_element_default(AACContext *ac, int channels)
> +{
> + program_config_struct pcs;
> +
> + memset(&pcs, 0, sizeof(program_config_struct));
> +
> + /* Premixed downmix outputs are not available */
> + pcs.mono_mixdown = -1;
> + pcs.stereo_mixdown = -1;
> +
> + switch(channels) {
> + case 1: /* Mono */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + break;
> +
> + case 2: /* Stereo */
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + break;
> +
> + case 3: /* Front Center + L + R */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + break;
> +
> + case 4: /* Front Center + L + R + Back Center */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + pcs.sce_type[1] = AAC_CHANNEL_BACK;
> + break;
> +
> + case 5: /* Front Center + L + R + Back Stereo */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[1] = AAC_CHANNEL_BACK;
> + break;
> +
> + case 6: /* Front Center + L + R + Back Stereo + LFE */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[1] = AAC_CHANNEL_BACK;
> + pcs.lfe_type[0] = AAC_CHANNEL_LFE;
> + break;
> +
> + case 7: /* Front Center + L + R + Outer Front Left + Outer Front Right + Back Stereo + LFE */
> + pcs.sce_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[0] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[1] = AAC_CHANNEL_FRONT;
> + pcs.cpe_type[2] = AAC_CHANNEL_BACK;
> + pcs.lfe_type[0] = AAC_CHANNEL_LFE;
> + break;
> +
> + default:
> + av_log(ac->avccontext, AV_LOG_ERROR, "Invalid default channel configuration (%d channels)\n",
> + channels);
> + return -1;
> + }
Above is full of code duplication and can be done simpler.
[...]
> +/**
> + * Parse GA specific configuration
What is GA? I think things should not be abbreviated in comments if avoidable.
> + * reference: Table 4.1
> + */
> +static int GASpecificConfig(AACContext * ac, GetBitContext * gb, int channels) {
> + int ext = 0;
> +
> + if(get_bits1(gb)) { // frameLengthFlag
> + av_log(ac->avccontext, AV_LOG_ERROR, "960/120 MDCT window is not supported\n");
> + return -1;
> + }
> +
> + if (get_bits1(gb)) // dependsOnCoreCoder
> + skip_bits(gb, 14); // coreCoderDelay
> + ext = get_bits1(gb);
the ext=0 init above is useless
> +
> + if(ac->m4ac.object_type == AOT_AAC_SCALABLE ||
> + ac->m4ac.object_type == AOT_ER_AAC_SCALABLE)
> + skip_bits(gb, 3); // layerNr
> +
> + if (channels == 0) {
> + skip_bits(gb, 4); // element_instance_tag
> + if(program_config_element(ac, gb) < 0)
> + return -1;
> + } else {
> + if(program_config_element_default(ac, channels) < 0)
> + return -1;
> + }
> +
> + if (ext) {
> + switch (ac->m4ac.object_type) {
> + case AOT_ER_BSAC:
> + get_bits(gb, 5); // numOfSubFrame
> + get_bits(gb, 11); // layer_length
skip_bits()
[...]
> +/**
> + * Parse audio specific configuration
> + * reference: Table 1.13
> + */
> +static int AudioSpecificConfig(AACContext * ac, void *data, int data_size) {
> + GetBitContext gb;
> + int i;
> +
> + init_get_bits(&gb, data, data_size * 8);
> +
> + if((i = ff_mpeg4audio_get_config(&ac->m4ac, data, data_size)) < 0)
> + return -1;
> +
> + skip_bits_long(&gb, i);
> +
> + switch (ac->m4ac.object_type) {
> + case AOT_AAC_LC:
> +#ifdef AAC_SSR
> + case AOT_AAC_SSR:
> +#endif /* AAC_SSR */
> +#ifdef AAC_LTP
> + case AOT_AAC_LTP:
> +#endif /* AAC_LTP */
please remove the /* ... */, they really dont make this more
readable
[...]
> + if (dim == 2) {
> + for (j = 0; j < values * dim; j += dim) {
> + index = j/dim;
> + ac->vq[i][j ] = (index / (mod ) - off); index %= mod;
> + ac->vq[i][j+1] = (index - off);
> + }
> + } else {
> + for (j = 0; j < values * dim; j += dim) {
> + index = j/dim;
> + ac->vq[i][j ] = (index / (mod * mod * mod) - off); index %= mod*mod*mod;
> + ac->vq[i][j+1] = (index / (mod * mod ) - off); index %= mod*mod;
> + ac->vq[i][j+2] = (index / (mod ) - off); index %= mod;
> + ac->vq[i][j+3] = (index - off);
> + }
> + }
code duplication, also this can be implemented simpler
> + }
> +
> + dsputil_init(&ac->dsp, avccontext);
> +
> + /* Initialize RNG dither */
> + av_init_random(0x1f2e3d4c, &ac->random_state);
> +
> + // -1024 - compensate wrong IMDCT method
> + // 32768 - values in AAC build for ready float->int 16 bit audio, using
> + // BIAS method instead needs values -1<x<1
> + for (i = 0; i < 256; i++)
> + ac->intensity_tab[i] = pow(0.5, (i - 100) / 4.);
> + for (i = 0; i < sizeof(ac->ivquant_tab)/sizeof(ac->ivquant_tab[0]); i++)
> + ac->ivquant_tab[i] = pow(i, 4./3);
> +
> + if(ac->dsp.float_to_int16 == ff_float_to_int16_c) {
> + ac->add_bias = 385.0f;
> + ac->sf_scale = 1. / (-1024. * 32768.);
> + } else {
> + ac->add_bias = 0.0f;
> + ac->sf_scale = 1. / -1024.;
> + }
> + for (i = 0; i < 256; i++)
> + ac->pow2sf_tab[i] = pow(2, (i - 100)/4.) * ac->sf_scale;
> +
> + if(init_vlc(&ac->mainvlc, 7, sizeof(code)/sizeof(code[0]),
> + bits, sizeof(bits[0]), sizeof(bits[0]),
> + code, sizeof(code[0]), sizeof(code[0]),
> + 0) < 0)
> + return -1;
> +
> +#ifdef AAC_SSR
> + if (ac->audioObjectType == AOT_AAC_SSR) {
> + ff_mdct_init(&ac->mdct, 9, 1);
> + ff_mdct_init(&ac->mdct_small, 6, 1);
> + // windows init
> + ff_kbd_window_init(ac->kbd_long_1024, 4.0, 256);
> + ff_kbd_window_init(ac->kbd_short_128, 6.0, 32);
> + sine_window_init(ac->sine_long_1024, 512);
> + sine_window_init(ac->sine_short_128, 64);
> + ssr_context_init(&ac->ssrctx);
> + } else {
> +#endif /* AAC_SSR */
> + ff_mdct_init(&ac->mdct, 11, 1);
> + ff_mdct_init(&ac->mdct_small, 8, 1);
> + // windows init
> + ff_kbd_window_init(ac->kbd_long_1024, 4.0, 1024);
> + ff_kbd_window_init(ac->kbd_short_128, 6.0, 128);
> + sine_window_init(ac->sine_long_1024, 2048);
> + sine_window_init(ac->sine_short_128, 256);
There are 4 different sine windows, why are they duplicated in each context?
They should be in static arrays generated at runtime or static const tables
hardcoded depening on ENABLE_HARDCODED_TABLES. This applies likely to other
tables as well.
[...]
> +/**
> + * Decode a data_stream_element
> + * reference: Table 4.10
> + */
> +static int data_stream_element(AACContext * ac, GetBitContext * gb, int id) {
> + int byte_align;
> + int count;
> + byte_align = get_bits1(gb);
> + count = get_bits(gb, 8);
declaration and initialization can be merged
> + if (count == 255)
> + count += get_bits(gb, 8);
without checking the spec this is not meant to be a while show_bits==255 ?
[...]
> +#ifdef AAC_LTP
> +static void ltp_data(AACContext * ac, GetBitContext * gb, int max_sfb, ltp_struct * ltp) {
> + int sfb;
> + if (ac->audioObjectType == AOT_ER_AAC_LD) {
> + assert(0);
hmm
[...]
> +static inline float ivquant(AACContext * ac, int a) {
> + static const float sign[2] = { -1., 1. };
> + int tmp = (a>>31);
> + int abs_a = (a^tmp)-tmp;
> + if (abs_a < sizeof(ac->ivquant_tab)/sizeof(ac->ivquant_tab[0]))
> + return sign[tmp+1] * ac->ivquant_tab[abs_a];
What is the point of the sign splitout? it seems that it would be simpler
to have that in teh table as well
[...]
> +/**
> + * Decode section_data payload
> + * reference: Table 4.46
> + */
> +static int section_data(AACContext * ac, GetBitContext * gb, ics_struct * ics, int cb[][64]) {
> + int g;
> + for (g = 0; g < ics->num_window_groups; g++) {
> + int bits = (ics->window_sequence == EIGHT_SHORT_SEQUENCE) ? 3 : 5;
> + int k = 0;
> + while (k < ics->max_sfb) {
> + int sect_len = 0;
> + int sect_len_incr = 1;
> + int sect_cb = get_bits(gb, 4);
> + if (sect_cb == 12) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "Invalid code book\n");
> + return -1;
> + }
> + while ((sect_len_incr = get_bits(gb, bits)) == (1 << bits)-1)
> + sect_len += sect_len_incr;
> + sect_len += sect_len_incr;
> + sect_len += k;
> + for (; k < sect_len && k < ics->max_sfb; k++)
> + cb[g][k] = sect_cb;
> + assert(k == sect_len);
Is this assert() checking for a condition which can occur from a corrupted
stream? It looks like it is ...
[...]
> +/**
> + * Decode scale_factor_data
> + * reference: Table 4.47
> + */
> +static int scale_factor_data(AACContext * ac, GetBitContext * gb, float mix_gain, unsigned int global_gain, ics_struct * ics, const int cb[][64], float sf[][64]) {
Maybe rename the function to what the comment says
> + int g, i;
> + unsigned int intensity = 100; // normalization for intensity_tab lookup table
> + int noise = global_gain - 90;
> + int noise_flag = 1;
> + ics->intensity_present = 0;
> + for (g = 0; g < ics->num_window_groups; g++) {
> + for (i = 0; i < ics->max_sfb; i++) {
> + if (cb[g][i] == ZERO_HCB) {
> + sf[g][i] = 0.;
> + } else if (cb[g][i] == INTENSITY_HCB || cb[g][i] == INTENSITY_HCB2) {
> + ics->intensity_present = 1;
> + intensity += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> + if(intensity > 255) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Intensity (%d) out of range", intensity);
> + return -1;
> + }
> + sf[g][i] = ac->intensity_tab[intensity];
> + } else if (cb[g][i] == NOISE_HCB) {
> + if (noise_flag) {
> + noise_flag = 0;
> + noise += get_bits(gb, 9) - 256;
> + } else {
> + noise += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> + }
> + sf[g][i] = pow(2.0, 0.25 * noise) * ac->sf_scale;
> + } else {
> + global_gain += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> + if(global_gain > 255) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Global gain (%d) out of range", global_gain);
> + return -1;
> + }
> + sf[g][i] = ac->pow2sf_tab[global_gain];
> + }
The identical code is implemented here 3 times, 2 times with seperate
LUTs and once by directly calling pow()
The differenes look like bugs. like forgetting to check validity or
performing the scaling needed for the float2int16 transform.
if(cb[g][i] == NOISE_HCB && noise_flag-- > 0)
gain[index] += get_bits(gb, 9) - 256;
else
gain[index] += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
if(gain[index] > 255)
...
sf[g][i] = ac->pow2sf_tab[gain[index]];
[...]
> +static void pulse_data(AACContext * ac, GetBitContext * gb, pulse_struct * pulse) {
the name is not goof IMHO
> + int i;
> + pulse->num_pulse = get_bits(gb, 2);
> + pulse->start = get_bits(gb, 6);
> + for (i = 0; i <= pulse->num_pulse; i++) {
num_pulse sounds like the number of entries but it is the number of entries-1
this is slightly unintuitiv
[...]
> +static void tns_data(AACContext * ac, GetBitContext * gb, const ics_struct * ics, tns_struct * tns) {
the name is also not too good ...
[...]
> +#ifdef AAC_SSR
> +static int gain_control_data(AACContext * ac, GetBitContext * gb, sce_struct * sce) {
> + // wd_num wd_test aloc_size
> + static const int gain_mode[4][3] = {
> + {1, 0, 5}, //ONLY_LONG_SEQUENCE = 0,
> + {2, 1, 2}, //LONG_START_SEQUENCE,
> + {8, 0, 2}, //EIGHT_SHORT_SEQUENCE,
> + {2, 1, 5}, //LONG_STOP_SEQUENCE
> + };
uint8_t
gain_mode[x][1] == (x&1)
[...]
> +static int ms_data(AACContext * ac, GetBitContext * gb, cpe_struct * cpe) {
> + ms_struct * ms = &cpe->ms;
> + ms->present = get_bits(gb, 2);
> + if (ms->present == 1) {
> + int g, i;
> + for (g = 0; g < cpe->ch[0].ics.num_window_groups; g++)
> + for (i = 0; i < cpe->ch[0].ics.max_sfb; i++)
> + ms->mask[g][i] = get_bits1(gb);// << i;
> + } else if (ms->present == 2) {
> + int g, i;
> + for (g = 0; g < cpe->ch[0].ics.num_window_groups; g++)
> + for (i = 0; i < cpe->ch[0].ics.max_sfb; i++)
> + ms->mask[g][i] = 1;// = 0xFFFFFFFFFFFFFFFFULL;
memset() ?
> + }
> + return 0;
> +}
also this always returns 0 so it can be void
> +
> +/**
> + * Decode spectral data
> + * reference: Table 4.50
> + */
> +static int spectral_data(AACContext * ac, GetBitContext * gb, const ics_struct * ics, const int cb[][64], int * icoef) {
> + static const int unsigned_cb[] = { 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1 };
unsigned_cb[x] == x&10
> + int i, k, g;
> + const uint16_t * offsets = ics->swb_offset;
> +
> + for (g = 0; g < ics->num_window_groups; g++) {
> + for (i = 0; i < ics->max_sfb; i++) {
> + const int cur_cb = cb[g][i];
> + const int dim = cur_cb >= FIRST_PAIR_HCB ? 2 : 4;
> + int group;
> + if (cur_cb == INTENSITY_HCB2 || cur_cb == INTENSITY_HCB) {
> + continue;
> + }
> + if (cur_cb == NOISE_HCB) {
> + for (group = 0; group < ics->group_len[g]; group++) {
> + for (k = offsets[i]; k < offsets[i+1]; k++)
> + icoef[group*128+k] = av_random(&ac->random_state) & 0x0000FFFF;
> + }
> + continue;
> + }
> + if (cur_cb == ZERO_HCB) {
> + for (group = 0; group < ics->group_len[g]; group++) {
> + memset(icoef + group * 128 + offsets[i], 0, (offsets[i+1] - offsets[i])*sizeof(int));
> + }
> + continue;
> + }
if / else if / else should look nicer than these continues
[...]
> + while (get_bits1(gb)) n++;
> + ptr[j] = (1<<n) + get_bits(gb, n);
this is defnitly not valid, get_bits can not read arbitrary amounts
> + }
> + }
> + }
> + for (j = 0; j < dim; j++)
> + icoef[group*128+k+j] = sign[j] * ptr[j];
This is really ugly, so much unneeded code, reading signs in a temporary array
copying values and then multplying and throwig the intermediate arrays away.
> + }
> + assert(k == offsets[i+1]);
is this testing something impossible or bitstream correctness?
[...]
> +static int pulse_tool(AACContext * ac, const ics_struct * ics, const pulse_struct * pulse, int * icoef) {
> + int i, off;
> + if (pulse->present) {
> + if (ics->window_sequence == EIGHT_SHORT_SEQUENCE) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "Pulse tool not allowed in EIGHT SHORT SEUQUENCE\n");
> + return -1;
> + }
> + off = ics->swb_offset[pulse->start];
> + for (i = 0; i <= pulse->num_pulse; i++) {
> + off += pulse->offset[i];
> + if (icoef[off] > 0)
> + icoef[off] += pulse->amp[i];
> + else
> + icoef[off] -= pulse->amp[i];
> + }
> + }
> + return 0;
> +}
the if(pulse->present) can be checked much nicer by the caller
also i suspect the EIGHT_SHORT_SEQUENCE combination should be checked
somewhere else, like when the value is read from the bitstream.
> +
> +// Tools implementation
> +static void quant_to_spec_tool(AACContext * ac, const ics_struct * ics, const int * icoef, const int cb[][64], const float sf[][64], float * coef) {
is the comment related to the function?
[...]
> + if (cb[g][i] == NOISE_HCB) {
> + for (group = 0; group < ics->group_len[g]; group++) {
> + float energy = 0;
> + float scale = 1.;// / (float)(offsets[i+1] - offsets[i]);
> + for (k = offsets[i]; k < offsets[i+1]; k++)
> + energy += (float)icoef[group*128+k] * icoef[group*128+k];
> + scale *= sf[g][i] / sqrt(energy);
> + for (k = offsets[i]; k < offsets[i+1]; k++)
> + coef[group*128+k] = icoef[group*128+k] * scale;
> + }
Is this normalizing the energy of our random number generator? What is this
supposed to be good for?!
[...]
> +/**
> + * Decode an individual_channel_stream payload
> + * reference: Table 4.44
> + */
> +static int individual_channel_stream(AACContext * ac, GetBitContext * gb, int common_window, int scale_flag, sce_struct * sce) {
The function names are not good at all.
decode_ics_payload() being a much better choice.
[...]
> + if (spectral_data(ac, gb, ics, sce->cb, icoeffs) < 0)
> + return -1;
> + if (pulse_tool(ac, ics, &pulse, icoeffs) < 0)
> + return -1;
> + quant_to_spec_tool(ac, ics, icoeffs, sce->cb, sce->sf, out);
i wonder if things would be faster when these where merged
[...]
> +static void intensity_tool(AACContext * ac, cpe_struct * cpe) {
> + const ics_struct * ics = &cpe->ch[1].ics;
> + sce_struct * sce1 = &cpe->ch[1];
> + float *coef0 = cpe->ch[0].coeffs, *coef1 = cpe->ch[1].coeffs;
> + if (ics->intensity_present) {
> + const uint16_t * offsets = ics->swb_offset;
> + int g, gp, i, k;
> + int c;
> + float scale;
> + for (g = 0; g < ics->num_window_groups; g++) {
> + for (gp = 0; gp < ics->group_len[g]; gp++) {
> + for (i = 0; i < ics->max_sfb; i++) {
> + if (sce1->cb[g][i] == INTENSITY_HCB || sce1->cb[g][i] == INTENSITY_HCB2) {
> + c = -1 + 2 * (sce1->cb[g][i] - 14);
> + if (cpe->ms.present)
> + c *= 1 - 2 * cpe->ms.mask[g][i];
c= sce1->cb[g][i] - 14;
if (cpe->ms.present)
c ^= cpe->ms.mask[g][i];
scale = (2*c-1) * sce1->sf[g][i];
but iam not really sure if this is worth to avoid a if() scale= -scale;
> + scale = c * sce1->sf[g][i];
> + for (k = offsets[i]; k < offsets[i+1]; k++) {
> + coef1[k] = scale * coef0[k];
> + }
superflous {}
[...]
> +/**
> + * Decode a channel_pair_element
> + * reference: Table 4.4
> + */
> +static int channel_pair_element(AACContext * ac, GetBitContext * gb, int id) {
function name ....
[...]
> + // M/S tool
> + if (cpe->common_window) {
> + ms_tool(ac, cpe);
> + }
useless comment
[...]
> +static int coupling_channel_element(AACContext * ac, GetBitContext * gb, int id) {
> + float cc_scale[] = {
> + pow(2, 1/8.), pow(2, 1/4.), pow(2, 0.5), 2.
> + };
static const
> + int num_gain = 0;
> + int c, g, sfb;
> + int sign;
> + float scale;
> + sce_struct * sce;
> + coupling_struct * coup;
> + if (!ac->che_cc[id]) {
> + return -1;
> + }
somehow i think all these checks can be factored out.
> + sce = &ac->che_cc[id]->ch;
> + sce->mixing_gain = 1.0;
> +
> + coup = &ac->che_cc[id]->coup;
> +
> + coup->ind_sw = get_bits1(gb);
> + //if (coup->ind_sw)
> + // av_log(ac->avccontext, AV_LOG_ERROR, "aac: independently switched coupling\n");
> + coup->num_coupled = get_bits(gb, 3);
> + for (c = 0; c <= coup->num_coupled; c++) {
> + num_gain++;
> + coup->is_cpe[c] = get_bits1(gb);
> + coup->tag_select[c] = get_bits(gb, 4);
> + if (coup->is_cpe[c]) {
> + coup->l[c] = get_bits1(gb);
> + coup->r[c] = get_bits1(gb);
> + if (coup->l[c] && coup->r[c])
> + num_gain++;
> + }
> + }
> + coup->domain = get_bits1(gb);
> + sign = get_bits(gb, 1);
> + scale = cc_scale[get_bits(gb, 2)];
> +
> + if (individual_channel_stream(ac, gb, 0, 0, sce))
> + return -1;
> +
> + for (c = 0; c < num_gain; c++) {
> + int cge = 1;
> + int gain = 0;
> + float gain_cache = 1.;
> + if (c != 0) {
> + cge = coup->ind_sw ? 1 : get_bits1(gb);
> + gain = cge ? get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60: 0;
> + gain_cache = pow(scale, (float)gain);
useless cast and cc_scale is useless too
> + }
> + for (g = 0; g < sce->ics.num_window_groups; g++)
> + for (sfb = 0; sfb < sce->ics.max_sfb; sfb++)
> + if (sce->cb[g][sfb] == ZERO_HCB) {
> + coup->gain[c][g][sfb] = 0;
> + } else {
> + if (cge) {
> + coup->gain[c][g][sfb] = gain_cache;
> + } else {
> + if (sign) {
> + int t = get_vlc2(gb, ac->mainvlc.table, 7, 3);
> + int s = 1 - 2 * (t & 0x1);
> + gain += (t >> 1) - 30;
> + coup->gain[c][g][sfb] = s * pow(scale, (float)gain);
> + } else {
> + gain += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> + coup->gain[c][g][sfb] = pow(scale, (float)gain);
> + }
this can factorized somewhat
[...]
> +static int excluded_channels(AACContext * ac, GetBitContext * gb) {
> + int i;
> + int n = 0;
> + int num_excl_chan = 7;
> +
> + for (i = 0; i < 7; i++)
> + ac->che_drc->exclude_mask[i] = get_bits1(gb);
> + n++;
?
n=0;
n++;
?
> +
> + while (get_bits1(gb)) {
> + ac->che_drc->additional_excluded_chns[n-1]=1;
> + for (i = num_excl_chan; i < num_excl_chan+7; i++)
> + ac->che_drc->exclude_mask[i] = get_bits1(gb);
> + n++;
> + num_excl_chan += 7;
> + }
This looks exploitable, also when we are already at it, please make sure
there are no other such cases in your code. Also make sure the decoder
doesnt crash with damaged input (see tools/trasher or other programs).
[...]
> +static int dynamic_range_info(AACContext * ac, GetBitContext * gb, int cnt) {
> + int n = 1;
> + int drc_num_bands = 1;
> + int i;
> +
> + if (!ac->che_drc)
> + ac->che_drc = av_mallocz(sizeof(drc_struct));
Why is this malloced() and not always part of the context?
[...]
> +static void tns_filter_tool(AACContext * ac, int decode, sce_struct * sce, float * coef) {
bad name ...
> + const ics_struct * ics = &sce->ics;
> + const tns_struct * tns = &sce->tns;
> + const int mmm = FFMIN(ics->tns_max_bands, ics->max_sfb);
> + int w, filt, m, i, ib;
> + int bottom, top, order, start, end, size, inc;
> + float tmp;
> + float lpc[TNS_MAX_ORDER + 1], b[2 * TNS_MAX_ORDER];
> + if (!tns->present) return;
this would be much cleaner where its called
> +
> + for (w = 0; w < ics->num_windows; w++) {
> + bottom = ics->num_swb;
> + for (filt = 0; filt < tns->n_filt[w]; filt++) {
> + top = bottom;
> + bottom = top - tns->length[w][filt];
> + if (bottom < 0)
> + bottom = 0;
FFMAX
[...]
> + if (decode) {
> + for (m = 0; m < size; m++) {
> + tmp = coef[start];
> + for (i = 0; i < order; i++)
> + tmp -= b[ib + i] * lpc[i + 1];
> + if (--ib < 0)
> + ib = order - 1;
> + b[ib] = b[ib + order] = tmp;
> + coef[start] = tmp;
> + start += inc;
> + }
> + } else { // encode
> + for (m = 0; m < size; m++) {
> + tmp = coef[start];
> + for (i = 0; i < order; i++)
> + tmp += b[i] * lpc[i + 1];
> + if (--ib < 0)
> + ib = order - 1;
> + b[ib] = b[ib + order] = tmp;
> + coef[start] = tmp;
> + start += inc;
> + }
> + }
Factorize the common code out please, "decode" is a constant when this
is inlined thus it will be optimized out anyway
> + }
> + }
> +}
> +
> +static void tns_trans(AACContext * ac, sce_struct * sce) {
> + tns_filter_tool(ac, 1, sce, sce->coeffs);
> +}
useless wraper?
> +
> +#ifdef AAC_LTP
> +static void window_ltp_tool(AACContext * ac, sce_struct * sce, float * in, float * out) {
> + ics_struct * ics = &sce->ics;
> + const float * lwindow = ics->window_shape ? ac->kbd_long_1024 : ac->sine_long_1024;
> + const float * swindow = ics->window_shape ? ac->kbd_short_128 : ac->sine_short_128;
> + const float * lwindow_prev = ics->window_shape_prev ? ac->kbd_long_1024 : ac->sine_long_1024;
> + const float * swindow_prev = ics->window_shape_prev ? ac->kbd_short_128 : ac->sine_short_128;
> + float * buf = ac->buf_mdct;
> + int i;
> + assert(ics->window_sequence != EIGHT_SHORT_SEQUENCE);
> + if (!ac->mdct_ltp) {
> + ac->mdct_ltp = av_malloc(sizeof(MDCTContext));
> + ff_mdct_init(ac->mdct_ltp, 11, 0);
> + }
Belongs into the init code
> + if (ics->window_sequence != LONG_STOP_SEQUENCE) {
> + vector_fmul_dst(ac, buf, in, lwindow_prev, 1024);
> + } else {
> + memset(buf, 0, 448 * sizeof(float));
> + for (i = 448; i < 576; i++) in[i] *= 0.125; // normalize
> + vector_fmul_dst(ac, buf + 448, in + 448, swindow_prev, 128);
> + memcpy(buf + 576, in + 576, 448 * sizeof(float));
> + }
> + if (ics->window_sequence != LONG_START_SEQUENCE) {
> + ac->dsp.vector_fmul_reverse(buf + 1024, in + 1024, lwindow, 1024);
> + } else {
> + memcpy(buf + 1024, in + 1024, 448 * sizeof(float));
> + for (i = 448; i < 576; i++) in[i + 1024] *= 0.125; // normalize
> + ac->dsp.vector_fmul_reverse(buf + 1024 + 448, in + 1024 + 448, swindow, 128);
> + memset(buf + 1024 + 576, 0, 448 * sizeof(float));
> + }
> + ff_mdct_calc(ac->mdct_ltp, out, buf, in); // using in as buffer for mdct
indention
[...]
> +
> +
> +/**
> + * @todo: Replace this with float_to_int16()
> + */
> +static inline int16_t ltp_round(float x) {
yes, please replace it ....
> + if (x >= 0)
> + {
> + if (x >= 1.0f)
> + return 32767;
> + } else {
> + if (x <= -1.0f)
> + return -32768;
> + }
> +
> + return lrintf(32768 * x);
> +}
> +
> +
> +static void ltp_update_trans(AACContext * ac, sce_struct * sce) {
> + int i;
> + if (!sce->ltp_state)
> + sce->ltp_state = av_mallocz(4 * 1024 * sizeof(int16_t));
belongs to some init()
> + if (ac->is_saved) {
> + for (i = 0; i < 1024; i++) {
> + sce->ltp_state[i] = sce->ltp_state[i + 1024];
> + sce->ltp_state[i + 1024] = ltp_round(sce->ret[i] - ac->add_bias);
> + sce->ltp_state[i + 2 * 1024] = ltp_round(sce->saved[i]);
> + //sce->ltp_state[i + 3 * 1024] = 0;
> + }
> + }
> +}
> +#endif /* AAC_LTP */
> +
> +static void window_trans(AACContext * ac, sce_struct * sce) {
> + ics_struct * ics = &sce->ics;
> + float * in = sce->coeffs;
> + float * out = sce->ret;
> + float * saved = sce->saved;
> + const float * lwindow = ics->window_shape ? ac->kbd_long_1024 : ac->sine_long_1024;
> + const float * swindow = ics->window_shape ? ac->kbd_short_128 : ac->sine_short_128;
> + const float * lwindow_prev = ics->window_shape_prev ? ac->kbd_long_1024 : ac->sine_long_1024;
> + const float * swindow_prev = ics->window_shape_prev ? ac->kbd_short_128 : ac->sine_short_128;
> + float * buf = ac->buf_mdct;
> + int i;
> +
> + if (ics->window_sequence != EIGHT_SHORT_SEQUENCE) {
> + ff_imdct_calc(&ac->mdct, buf, in, out); // out can be abused for now as a temp buffer
> + if (ac->is_saved) {
> + if (ics->window_sequence != LONG_STOP_SEQUENCE) {
> + ac->dsp.vector_fmul_add_add(out, buf, lwindow_prev, saved, ac->add_bias, 1024, 1);
> + } else {
> + for (i = 0; i < 448; i++) out[i] = saved[i] + ac->add_bias;
> + for (i = 448; i < 576; i++) buf[i] *= 0.125; // normalize
> + ac->dsp.vector_fmul_add_add(out + 448, buf + 448, swindow_prev, saved + 448, ac->add_bias, 128, 1);
> + for (i = 576; i < 1024; i++) out[i] = buf[i] + saved[i] + ac->add_bias;
> + }
> + }
> + if (ics->window_sequence != LONG_START_SEQUENCE) {
> + ac->dsp.vector_fmul_reverse(saved, buf + 1024, lwindow, 1024);
> + } else {
> + memcpy(saved, buf + 1024, 448 * sizeof(float));
> + for (i = 448; i < 576; i++) buf[i + 1024] *= 0.125; // normalize
> + ac->dsp.vector_fmul_reverse(saved + 448, buf + 1024 + 448, swindow, 128);
> + memset(saved + 576, 0, 448 * sizeof(float));
thats alot of copying around, cant that be avoided?
also the whole looks like theres some code duplication to window_ltp_tool()
and window_ssr_tool()
[...]
> +static void coupling_dependent_trans(AACContext * ac, cc_struct * cc, sce_struct * sce, int index) {
> + ics_struct * ics = &cc->ch.ics;
> + const uint16_t * offsets = ics->swb_offset;
> + float * dest = sce->coeffs;
> + float * src = cc->ch.coeffs;
> + int g, i, group, k;
> + if(ac->m4ac.object_type == AOT_AAC_LTP) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Dependent coupling is not supported together with LTP\n");
> + return;
> + }
> + for (g = 0; g < ics->num_window_groups; g++) {
> + for (i = 0; i < ics->max_sfb; i++) {
> + if (cc->ch.cb[g][i] != ZERO_HCB) {
> + float gain = cc->coup.gain[index][g][i] * sce->mixing_gain;
> + for (group = 0; group < ics->group_len[g]; group++) {
> + for (k = offsets[i]; k < offsets[i+1]; k++) {
> + dest[group*128+k] += gain * src[group*128+k];
> + }
> + }
> + }
> + }
> + dest += ics->group_len[g]*128;
> + src += ics->group_len[g]*128;
vertical align
[...]
> +static int aac_decode_frame(AVCodecContext * avccontext, void * data, int * data_size, const uint8_t * buf, int buf_size) {
> + AACContext * ac = avccontext->priv_data;
> + GetBitContext gb;
> + int id, err, tag;
you are not checking that there is enough space in "data".
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I wish the Xiph folks would stop pretending they've got something they
do not. Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20080402/2acd82f1/attachment.pgp>
More information about the ffmpeg-devel
mailing list