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

Michael Niedermayer michaelni
Fri May 15 13:03:44 CEST 2009


On Fri, May 15, 2009 at 10:52:05AM +0300, Siarhei Siamashka wrote:
> On Thursday 14 May 2009, Michael Niedermayer wrote:
> > On Thu, May 14, 2009 at 12:09:31PM +0300, Siarhei Siamashka wrote:
> > > On Thu, May 14, 2009 at 11:29 AM, 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.
> > >
> > > Everything is calculated in double precision floating point, with the
> > > final results stored in single precision. Additionally, transcendental
> > > functions generally have their precision unspecified unless I'm
> > > mistaken. There are plenty of extra bits of precision in intermediate
> > > calculations. Regression test does not detect any problems, this
> > > probably means that accumulated rounding errors never had any real
> > > impact and the final result of double->float conversion was always the
> > > same (of course it could affect the results theoretically, but this
> > > probability is rather low).
> >
> > We did spend enough time debuging cos/sin rounding errors, in the past
> > already iam not going to take any chances when theres no benefit from it at
> > all
> 
> Yes, I know this. Also "cos(x)" is not exactly equal to "sin(x + 0.5 * M_PI)"
> in practice. So there is still a small theoretical chance that having scale
> factor set to "-1.0" can produce results not completely equal to using scale
> factor "1.0" initially and changing sign of IMDCT output manually. This does
> not seem to happen in tests with real audio files though (and this does not
> happen even with my test variant of the code quoted above, which had precision
> for intermediate calculations intentionally relaxed).
> 
> Would this patch be finally OK (kind of blend of your and Reimar's
> suggestions)?
[...]

> @@ -85,10 +85,12 @@ av_cold int ff_mdct_init(MDCTContext *s, int nbits, int inverse)
>      if (!s->tsin)
>          goto fail;
>  
> +    theta = scale < 0 ? 0.5 * M_PI : 0;
> +    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);
> +        alpha = 2 * M_PI * (i + 1.0 / 8.0) / n + theta;

alpha = 2 * M_PI * (i + theta) / n;

would be slightly more accurate as (i + theta) / n is exact and thus
the addition of two inexact variables is avoided


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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/20090515/32b0e613/attachment.pgp>



More information about the ffmpeg-devel mailing list