[FFmpeg-devel] AAC decoder round 6

[Robert Swain]

2008/8/10 Robert Swain <robert.swain at gmail.com>:
> 2008/8/9 Michael Niedermayer <michaelni at gmx.at>:
>> On Sat, Aug 09, 2008 at 12:25:09PM +0100, Robert Swain wrote:
>>> +            ac->dsp.vector_fmul_reverse(saved, buf + 1024, lwindow, 1024);
>>> +        } else {
>>> +            memcpy(saved, buf + 1024, 448 * sizeof(float));
>>> +            ac->dsp.vector_fmul_reverse(saved + 448, buf + 1024 + 448, swindow, 128);
>>> +            memset(saved + 576, 0, 448 * sizeof(float));
>>> +        }
>>> +    } else {
>>> +        if (ics->window_sequence[1] == ONLY_LONG_SEQUENCE || ics->window_sequence[1] == LONG_STOP_SEQUENCE)
>>> +            av_log(ac->avccontext, AV_LOG_WARNING,
>>> +                   "Transition from an ONLY_LONG or LONG_STOP to an EIGHT_SHORT sequence detected. "
>>> +                   "If you heard an audible artifact, please submit the sample to the FFmpeg developers.\n");
>>> +        for (i = 0; i < 2048; i += 256) {
>>> +            ff_imdct_calc(&ac->mdct_small, buf + i, in + i/2, out);
>>> +            ac->dsp.vector_fmul_reverse(ac->revers + i/2, buf + i + 128, swindow, 128);
>>> +        }
>>> +        for (i = 0; i < 448; i++)   out[i] = saved[i] + ac->add_bias;
>>> +
>>> +        ac->dsp.vector_fmul_add_add(out + 448 + 0*128, buf + 0*128, swindow_prev, saved + 448 ,       ac->add_bias, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(out + 448 + 1*128, buf + 2*128, swindow,      ac->revers + 0*128, ac->add_bias, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(out + 448 + 2*128, buf + 4*128, swindow,      ac->revers + 1*128, ac->add_bias, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(out + 448 + 3*128, buf + 6*128, swindow,      ac->revers + 2*128, ac->add_bias, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(out + 448 + 4*128, buf + 8*128, swindow,      ac->revers + 3*128, ac->add_bias,  64, 1);
>>> +
>>> +#if 0
>>> +        vector_fmul_add_add_add(&ac->dsp, out + 448 + 1*128, buf + 2*128, swindow,      saved + 448 + 1*128, ac->revers + 0*128, ac->add_bias, 128);
>>> +        vector_fmul_add_add_add(&ac->dsp, out + 448 + 2*128, buf + 4*128, swindow,      saved + 448 + 2*128, ac->revers + 1*128, ac->add_bias, 128);
>>> +        vector_fmul_add_add_add(&ac->dsp, out + 448 + 3*128, buf + 6*128, swindow,      saved + 448 + 3*128, ac->revers + 2*128, ac->add_bias, 128);
>>> +        vector_fmul_add_add_add(&ac->dsp, out + 448 + 4*128, buf + 8*128, swindow,      saved + 448 + 4*128, ac->revers + 3*128, ac->add_bias, 64);
>>> +#endif
>>> +
>>> +        ac->dsp.vector_fmul_add_add(saved,       buf + 1024 + 64,    swindow + 64, ac->revers + 3*128+64,  0, 64, 1);
>>> +        ac->dsp.vector_fmul_add_add(saved + 64,  buf + 1024 + 2*128, swindow,      ac->revers + 4*128,     0, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(saved + 192, buf + 1024 + 4*128, swindow,      ac->revers + 5*128,     0, 128, 1);
>>> +        ac->dsp.vector_fmul_add_add(saved + 320, buf + 1024 + 6*128, swindow,      ac->revers + 6*128,     0, 128, 1);
>>> +        memcpy(                     saved + 448, ac->revers + 7*128, 128 * sizeof(float));
>>
>>> +        memset(                     saved + 576, 0,                  448 * sizeof(float));
>>
>> is that really needed? I mean if the data isnt used it wouldnt be but if its
>> used then windowing and adding zeros seems rather like wasted cpu time.
>> also this applies to all the memset in thus function.
>
> In some cases these zeros are being added. Working around it using
> logic based on the window sequences is mostly OK until
> window_sequence[0] is not eight short and not long stop. That is when
> this code is encountered:
>
> ac->dsp.vector_fmul_add_add(out, buf, lwindow_prev, saved,
> ac->add_bias, 1024, 1);
>
> It seems a bit weird to make this code read as attached
> (20080810-1202-windowing_avoid_adding_zeros.diff).
>
> When porting to imdct_half(), Loren suggested that I focus on the
> transitions and assume there are only long-to-long or short-to-short
> transitions and any long-to-short or short-to-long transitions are
> treated as short-to-short. This simplifies the cases significantly. I
> can make a patch to do this if desirable. In fact, this could be the
> first stage of changes to imdct_and_windowing() when porting to
> imdct_half() after all this is in trunk, if you wish.
>
> I intend to revisit my imdct_half() porting efforts shortly but I
> won't work on that right now. I had it working but there was a bug so
> it would screech for a moment then play fine then screech again at
> irregular intervals. So I think I got one or more of the cases wrong.

Any comment on this one? I'm looking at the windowing code at the moment.

Regards,
Rob