[FFmpeg-cvslog] r26033 - trunk/libavcodec/ac3enc.c

[Justin Ruggles]

On 12/19/2010 01:17 PM, Justin Ruggles wrote:

> On 12/18/2010 05:18 PM, Vitor Sessak wrote:
>> On 12/16/2010 04:06 PM, jbr wrote:
>>> Author: jbr
>>> Date: Thu Dec 16 16:06:28 2010
>>> New Revision: 26033
>>>
>>> Log:
>>> Use optimized function DSPContext.sad[0]() instead of calc_exp_diff().
>>> 90% faster compute_exp_strategy().
>>
>> This looks to have broken FATE, see http://fate.ffmpeg.org/x86.os2.444
>> or http://fate.arrozcru.org/history.cgi?slot=x86_32-netbsd-gcc .
> 
> Thanks for letting me know.  I'll test all the versions I can when I get 
> back home tomorrow to see if I can track down the issue.  It would be 
> good to know which version the affected systems are using.  Does fate 
> save config.h?


I'm not sure what could be causing this problem.  All BSD and OS/2 seem
to have issues with this change.  There is nothing I can find other than
compiler or system problems that would lead to this.  It appears to be
something seriously wrong though because the output size is 1405 as
opposed to 98751, but there are no errors logged during encoding.

All of the configurations which failed after this revision are x86.  I
tested all possible x86 versions of this function and got passing tests
on my machine (x86_64 linux gcc 4.4.3).  I'm not sure what to do at this
point.  Should I revert this commit since it apparently seriously breaks
AC-3 encoding on BSD?  This has a significant speed impact though.  I'm
almost tempted to install BSD so I can at least begin to track down what
the heck is going on.

Unrelated to this (but still concerning FATE failures), the ARM and
AVR32 configurations did not like the previous ac3 encoder changes.  The
output size is the same, but the checksums differ.  My best guess is
that it is related to the lrintf() in the float-to-int conversion for
the sin/cos tables.  I could remove the lrintf() but I don't think it's
a good idea.  Alternatively, I could hardcode the tables like is done
for the MDCT window and that would probably fix it.

-Justin