[FFmpeg-soc] [soc]: r2719 - nellyenc/nellymoserenc.c

[Bartlomiej Wolowiec]

On środa, 9 lipca 2008, Michael Niedermayer wrote:
> On Tue, Jul 08, 2008 at 11:17:06PM +0200, Bartlomiej Wolowiec wrote:
> > On poniedziałek, 7 lipca 2008, Michael Niedermayer wrote:
> > > On Mon, Jul 07, 2008 at 10:55:16PM +0200, Reimar Döffinger wrote:
> > > > Hello,
> > > >
> > > > On Mon, Jul 07, 2008 at 10:21:38PM +0200, Bartlomiej Wolowiec wrote:
> > > > > On poniedziałek, 7 lipca 2008, Robert Swain wrote:
> > > > > > 2008/7/7 bwolowiec <subversion at mplayerhq.hu>:
> > > > > > > Author: bwolowiec
> > > > > > > Date: Mon Jul  7 16:41:50 2008
> > > > > > > New Revision: 2719
> > > > > > >
> > > > > > > Log:
> > > > > > > use ff_sine_window_init
> > > > > > >
> > > > > > >
> > > > > > > Modified:
> > > > > > >   nellyenc/nellymoserenc.c
> > > > > > >
> > > > > > > Modified: nellyenc/nellymoserenc.c
> > > > > > > ===============================================================
> > > > > > >==== ====== ===== --- nellyenc/nellymoserenc.c    (original) +++
> > > > > > > nellyenc/nellymoserenc.c    Mon Jul  7 16:41:50 2008 @@ -116,10
> > > > > > > +116,13 @@ static av_cold int encode_init(AVCodecCo
> > > > > > > dsputil_init(&s->dsp, avctx);
> > > > > > >
> > > > > > >     /* Generate overlap window */
> > > > > > > -    if (!sine_window[0])
> > > > > > > -        for (i=0 ; i<256; i++) {
> > > > > > > -            sine_window[i] = sin((i + 0.5) / 256.0 * M_PI) /8;
> > > > > > > +    if (!sine_window[0]){
> > > > > > > +        ff_sine_window_init(sine_window, 128);
> > > > > > > +        for (i=0; i<128; i++) {
> > > > > > > +            sine_window[i] /= 8.0;
> > > > > >
> > > > > > This isn't thread safe. You must merge the /8 into some other
> > > > > > calculation.
> > > > >
> > > > > Hmm.. yes, you're right. Unfortunately, I don't see now where it
> > > > > can be calculated. If I don't calculate it in init I will have to
> > > > > calculate it additionally in every encode_tag, it isn't a good
> > > > > idea.  Maybe it's better to use previous version and don't use
> > > > > ff_sine_window_init...? or maybe (in my opinion a bit unnatural)
> > > > > such a solution:
> > > > > ff_sine_window_init(tmp_sine_window, 128);
> > > > > for (i=0; i<128; i++) {
> > > > >   sine_window[i] = tmp_sine_window[i] / 8.0;
> > > > >   sine_window[255-i] = sine_window[i];
> > > > > }
> > > >
> > > > Just use your original code on some per-thread buffer (stack or heap
> > > > depending on size) and then memcpy it over.
> > >
> > > Please everyone stop scaring me ...
> > >
> > > pval = -pow(2, pows[j]/2048);
> > > tmp = s->mdct_out[i*NELLY_BUF_LEN + j] / pval;
> > >
> > > above is in the current code, that will happily take a /8
> > > besides it obviously should be a * not a / pval, / is slow
> > >
> > > [...]
> >
> > Do you think of something like this?
>
> yes, next get rid of the pow() thats slow as well ...
>
> > Or maybe instead of this additional calculations it can be better
> > calculated once while initialization... ?
>
> initialization of what?
> The window is supposed to be shared between codecs and cannot be changed
> besides what additional calculations? "pows" are differntially coded
> adding 3*2048 at the begin really does not matter, especially if you
> do a pow()in the inner loop.
>
> [...]

After short break I will try to spend much time on writing nellymoser encoder.

I don't see exactly how to make it using 3*2048 values...
Can you give me a bigger clue how to do it?

-- 
Bartlomiej Wolowiec