[FFmpeg-devel] AAC-Main (round 2)

[Alex Converse]

On Sat, Nov 8, 2008 at 2:03 PM, Alex Converse <alex.converse at gmail.com>wrote:

> On Sat, Nov 8, 2008 at 12:48 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
>
>> On Fri, Nov 07, 2008 at 07:20:45PM -0500, Alex Converse wrote:
>> > Hi,
>> >
>> > Attached are a series of patches that implements AAC Main in FFmpeg.
>> These
>> > are very similar to the first round. The only major change was avoiding
>> > unnecessary type punning.
>> >
>> > While AAC-Main is rarely used, faad2 supports it, flash claims to
>> support it
>> > (I haven't tested this), and we claim to support it but do not.
>> >
>> > Notes:
>> > 1) Frequency domain prediction is described only in ISO/IEC 13818-7 not
>> in
>> > 14496-3.
>>
>> > 2) The prediction operation uses 16-bit floats, using 32-bit floats does
>> not
>> > give adequate accuracy so emulation routines to round to 16-bit are
>> > included.
>>
>> > 3) As only 16-bit floats are required it could be possible to store the
>> > prediction state with half the memory but I'm not sure how to approach
>> that
>> > situation without resorting to IEEE type punning.
>> >
>> > Regards,
>> >
>> > Alex Converse
>>
>> [...]
>> > +static void reset_predictor_group(PredictorState * ps, int group_num) {
>> > +    int i;
>> > +    if (group_num)
>> > +        for (i = group_num-1; i < MAX_PREDICTORS; i+=30)
>> > +            reset_predict_state(&ps[i]);
>> > +}
>>
>> i think it would be clearer if the if() was moved out of the function
>>
>
> ok
>
>
>>
>>
>> [...]
>> > @@ -786,6 +837,95 @@ static int decode_spectrum_and_dequant(AACContext *
>> ac, float coef[1024], GetBit
>> >      return 0;
>> >  }
>> >
>> > +static av_always_inline float flt16_round(float pf) {
>> > +    int exp;
>> > +    pf = frexp(pf, &exp);
>> > +    pf = ldexp(roundf(ldexp(pf, 8)), exp-8);
>> > +    return pf;
>> > +}
>> > +
>> > +static av_always_inline float flt16_even(float pf) {
>> > +    int exp;
>> > +    pf = frexpf(pf, &exp);
>> > +    pf = ldexp(rintf(ldexp(pf, 8)), exp-8);
>> > +    return pf;
>> > +}
>> > +
>> > +static av_always_inline float flt16_trunc(float pf) {
>> > +    int exp;
>> > +    pf = frexpf(pf, &exp);
>> > +    pf = ldexp(truncf(ldexp(pf, 8)), exp-8);
>> > +    return pf;
>> > +}
>> > +
>>
>> are these faster or slower than the code suggested in the spec?
>>
>
> The code suggested in the spec uses type puns for flt16_round and
> flt16_trunc.
>
> flt16_even as suggested in the spec casts back and forth between float,
> double, and int, uses floating point multiplies and divides rather than
> ldexpf (which can take advantage of IEEE-754 on sane platforms), and uses
> two ifs. I haven't benchmarked it, but I'm pretty sure mine is faster.
>
>
>>
>> > +static void predict(AACContext * ac, PredictorState * ps, float* coef,
>> int output_enable) {
>>
>> > +    const float a = 0.953125;
>>
>> 61.0/64 (could be in a comment too if the literal float is prefered ...)
>>
>>
>> > +    const float alpha = 0.90625;
>>
>> 29.0/32
>>
>
>
> ok, do you have any idea why those values were chosen? I suppose I could
> try to hunt down some of the original literature.
>
>
>>
>> > +
>> > +    float e0, e1;
>> > +    float pv;
>> > +
>> > +    float k1, k2;
>> > +
>> > +    if (ps->var0 <= 1)
>> > +        k1 = 0;
>> > +    else
>> > +        k1 = ps->cor0*flt16_even(a/ps->var0);
>> > +
>> > +    if (ps->var1 <= 1)
>> > +        k2 = 0;
>> > +    else
>> > +        k2 = ps->cor1*flt16_even(a/ps->var1);
>> > +
>> > +    pv = k1*ps->r0 + k2*ps->r1;
>> > +    pv = flt16_round(pv);
>> > +    if (output_enable)
>> > +        *coef += pv/-1024;
>> > +
>> > +    e0 = *coef*-1024;
>
>
This -1024 constant is no good. It's tied to MMX dsputil.


>
>> > +    e1 = e0-k1*ps->r0;
>> > +
>>
>> > +    ps->cor1 = alpha*ps->cor1 + ps->r1*e1;
>> > +    ps->var1 = alpha*ps->var1 + 0.5 * (ps->r1*ps->r1 + e1*e1);
>> > +    ps->cor0 = alpha*ps->cor0 + ps->r0*e0;
>> > +    ps->var0 = alpha*ps->var0 + 0.5 * (ps->r0*ps->r0 + e0*e0);
>> > +
>> > +    ps->r1 = a*(ps->r0-k1*e0);
>> > +    ps->r0 = a*e0;
>> > +
>>
>> > +    ps->r0   = flt16_trunc(ps->r0);
>> > +    ps->r1   = flt16_trunc(ps->r1);
>> > +    ps->cor0 = flt16_trunc(ps->cor0);
>> > +    ps->cor1 = flt16_trunc(ps->cor1);
>> > +    ps->var0 = flt16_trunc(ps->var0);
>> > +    ps->var1 = flt16_trunc(ps->var1);
>>
>> the flt16_trunc() calls could be done when the values are calculated
>>
>
> I had left that there in case the consensus was that we wanted to store
> thes
>
>
In addition to the changes discussed, I also fixed the non-MMX dsputil
scaling

[...]

Regards,
Alex Converse
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 00-aac-predict.diff
Type: text/x-diff
Size: 8422 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081110/be3ee6c3/attachment.diff>