[FFmpeg-devel] AAC decoder round 6

[Michael Niedermayer]

On Mon, Aug 11, 2008 at 12:46:29AM +0100, Robert Swain wrote:
> 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.

Ive no real comment, if you can improve things within short time that is
welcome but due to the uncommon time constraint with gsoc end and kostyas
encoder i do not want that you work on anything that would eat too much time.
Such things can be done when its in svn (theoretically at least ...)

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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/20080811/97980d0c/attachment.pgp>