[FFmpeg-devel] [PATCH] HE-AACv1 second revision

[Michael Niedermayer]

On Sat, Jan 30, 2010 at 05:33:00PM -0500, Alex Converse wrote:
> On Sat, Jan 30, 2010 at 5:32 PM, Alex Converse <alex.converse at gmail.com> wrote:
> > Notes:
> > *There are still several lroundf() calls that take all integer inputs.
> > If anyone has advice on how to do them in an integer only fashion. I
> > would love to hear it.

could you tell me what range the input values to the remaining 4 equations
have?

a few comments (no full review) below

[...]
> +/**
> + * Analysis QMF Bank (14496-3 sp04 p206)
> + *
> + * @param   x       pointer to the beginning of the first sample window
> + * @param   W       array of complex-valued samples split into subbands
> + */
> +static void sbr_qmf_analysis(DSPContext *dsp, FFTContext *fft, const float *in, float *x,
> +                             FFTComplex u[64], float W[2][32][32][2])
> +{
> +    int i, k, l;
> +    const uint16_t *revtab = fft->revtab;
> +    memcpy(W[0], W[1], sizeof(W[0]));
> +    memcpy(x    , x+1024, (320-32)*sizeof(x[0]));
> +    memcpy(x+288, in    ,     1024*sizeof(x[0]));
> +    x += 319;
> +    for (l = 0; l < 32; l++) { // numTimeSlots*RATE = 16*2 as 960 sample frames
> +                               // are not supported
> +        float z[320];
> +        for (i = 0; i < 320; i++)
> +            z[i] = x[-i] * sbr_qmf_window_ds[i];
> +        for (i = 0; i < 64; i++) {
> +            float f = z[i] + z[i + 64] + z[i + 128] + z[i + 192] + z[i + 256];


> +            u[revtab[i]].re = f * analysis_cos_pre[i];
> +            u[revtab[i]].im = f * analysis_sin_pre[i];
> +        }
> +        ff_fft_calc(fft, u);

cant this be done with a fft /dct /whatever of half that size?


[...]
> +/// High Frequency Generator (14496-3 sp04 p215)
> +static int sbr_hf_gen(AACContext *ac, SpectralBandReplication *sbr,
> +                      float X_high[64][40][2], float X_low[32][40][2], float (*alpha0)[2],
> +                      float (*alpha1)[2], float bw_array[2][5], uint8_t *t_env,
> +                      int bs_num_env)
> +{
> +    int i, x, l;
> +    int k = sbr->k[4];
> +    for (i = 0; i < sbr->num_patches; i++) {
> +        for (x = 0; x < sbr->patch_num_subbands[i]; x++, k++) {
> +            const int g = find_freq_subband(sbr->f_tablenoise, sbr->n_q + 1, k);
> +            const int p = sbr->patch_start_subband[i] + x;
> +
> +            if (g < 0) {
> +                av_log(ac->avccontext, AV_LOG_ERROR,
> +                       "ERROR : no subband found for frequency %d\n", k);
> +                return -1;
> +            }
> +

> +            for (l = t_env[0] << 1; l < t_env[bs_num_env] << 1; l++) {

i would suggest to avoid using l as variable because it looks so similar to 1
makes the code quite hard to read IMHO


[...]
> +///< window coefficients for analysis/synthesis QMF banks
> +static DECLARE_ALIGNED_16(float, sbr_qmf_window_ds)[320];
> +static const DECLARE_ALIGNED_16(float, sbr_qmf_window_us)[640] = {

seems symmetric, thus maybe storing just half of it would work and it might
(or might not) allow some optimizations

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

Incandescent light bulbs waste a lot of energy as heat so the EU forbids them.
Their replacement, compact fluorescent lamps, much more expensive, dont fit in
many old lamps, flicker, contain toxic mercury, produce a fraction of the light
that is claimed and in a unnatural spectrum rendering colors different than
in natural light. Ah and we now need to turn the heaters up more in winter to
compensate the lower wasted heat. Who wins? Not the environment, thats for sure
-------------- 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/20100208/80a5f7a6/attachment.pgp>