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

[Michael Niedermayer]

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.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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-soc/attachments/20080709/4d20e7dd/attachment.pgp>