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

[Siarhei Siamashka]

On Sunday 10 May 2009, Reimar D?ffinger wrote:
> On Sun, May 10, 2009 at 12:38:17PM +0300, Siarhei Siamashka wrote:
> > This set of patches has been already OK'ed, but then was lost:
> > https://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-October/054266.htm
> >l
>
> Who OK'd id?

There is a reason why mailing list archives exist. I also placed
a link in my previous mail for your convenience :-)

> The ff_mdct_init change contains cosmetics and pointless code 
> duplication.

Could you elaborate? How can code duplication be avoided here without
obviously sacrificing performance (moving 'if' in the inner loop)?

> Also almost everyone uses a scale factor of 1, except vorbis,

Yes, but other codecs also may benefit from it later.

> wouldn't 
> it possibly make more sense to instead add a ff_mdct_init_scaled with the
> scale and leave the rest unchanged?

That was in my initial submission, ironically it had exactly the same function
name as you suggested :-) Here is also a link:
http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-September/053931.html

> I don't really know, just thinking. 
> I also wonder if it might be better if the calling code did the sqrt(),
> for the current uses doing it in ff_mdct_init seems like it would only
> add pointless complexity and possibly inaccuracies (or is e.g. sqrt(1.0) ==
> 1.0 always guaranteed by the C standard?).

Yes, I also felt that the use of additional '_scaled' variant was a reasonable
choice (either with some code duplication to avoid 'sqrt', or just calling one
function from another for now). The current patch passes regression tests for
all the codecs. For vorbis (which is still not covered by regression tests) it
caused a tiny difference in PSNR (the numbers also have been posted in the old
submission) because scaling gets actually used. In any case, floating point
math is not perfectly precise anyway.

-- 
Best regards,
Siarhei Siamashka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090510/36145ee4/attachment.pgp>