[FFmpeg-devel] [PATCH] Fix for inverted sign in ffvorbis?audio?output (resubmit)

[Reimar Döffinger]

On Thu, May 14, 2009 at 10:29:37AM +0200, Reimar D?ffinger wrote:
> On Thu, May 14, 2009 at 11:06:21AM +0300, Siarhei Siamashka wrote:
> > And what about this variant?
> >  
> > +    alpha = (scale < 0 ? 0.5 * M_PI : 0) + 0.25 * M_PI / n;
> > +    scale = -sqrt(fabs(scale));
> >      for(i=0;i<n4;i++) {
> > -        alpha = 2 * M_PI * (i + 1.0 / 8.0) / n;
> > -        s->tcos[i] = -cos(alpha);
> > -        s->tsin[i] = -sin(alpha);
> > +        s->tcos[i] = cos(alpha) * scale;
> > +        s->tsin[i] = sin(alpha) * scale;
> > +        alpha += 2.0 * M_PI / n;
> 
> Rounding errors accumulate like that, IMO you really should never do
> that unless a) you know the value you add can be represented exactly
> or b) you absolutely can't avoid it for performance reasons.

Note that I think you can get a) with

alpha = 0.125 / n;
if (scale < 0)
    alpha += 0.25;
for ....
cos(2*M_PI*alpha) * scale;
sin(2*M_PI*alpha) * scale;
alpha += 1.0 / n;

It doesn't really matter if you pull out M_PI or 2*M_PI, but I think
the latter is more logical.
Though I think it's questionable if all that should be done in a single
commit, IMO if we want that you should first change the way alpha is
calculated and then add the scale fix on top.