[FFmpeg-cvslog] ac3enc: use MUL64() to multiply fixed-point coefficients

[Justin Ruggles]

On 03/09/2011 02:05 PM, Michael Niedermayer wrote:

> On Wed, Mar 09, 2011 at 06:07:51PM +0000, M?ns Rullg?rd wrote:
>> Justin Ruggles <justin.ruggles at gmail.com> writes:
>>
>>> On 03/09/2011 12:30 PM, Michael Niedermayer wrote:
>>>
>>>> On Tue, Mar 08, 2011 at 06:18:44PM -0500, Justin Ruggles wrote:
>>>>> On 03/08/2011 02:01 PM, Justin Ruggles wrote:
>>>>>
>>>>>> ffmpeg | branch: master | Justin Ruggles <justin.ruggles at gmail.com> | Tue Mar  8 13:18:55 2011 -0500| [67fc32ac6197a7fe265795d0bf60bf704464394e] | committer: Michael Niedermayer
>>>>>>
>>>>>> ac3enc: use MUL64() to multiply fixed-point coefficients
>>>>>>
>>>>>> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>>>>>>
>>>>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=67fc32ac6197a7fe265795d0bf60bf704464394e
>>>>>> ---
>>>>>>
>>>>>>  libavcodec/ac3enc.c       |    8 ++++----
>>>>>>  libavcodec/ac3enc_fixed.h |    2 ++
>>>>>>  libavcodec/ac3enc_float.h |    2 ++
>>>>>>  3 files changed, 8 insertions(+), 4 deletions(-)
>>>>>>
>>>>>
>>>>>
>>>>> This change is unnecessary and can be safely reverted.
>>>>
>>>> i thought it improves speed on some cpus where MUL64 is optimized ?
>>>> but i can revert if you want me to
>>>
>>> Well, I wasn't thinking of that case.  The original purpose of the patch
>>> was only because it was necessary in earlier versions of my patch set
>>> where I was moving the rematrixing strategy decision to after the
>>> coefficient scaling.
>>>
>>> It might very well be faster though.  MAC64() would be another option.
>>> I'll work on that and send a separate patch.
>>
>> The inputs here are 16-bit so none of the existing macros are right.
> 
> The output of the MDCT should be more than 16bit.
> If thats not the case theres some quality that could be gained, at least at
> the high bitrate side IMHO


I'm not 100% sure about the full process, but it does appear to be
mathematically possible at first glance for it to be > 16-bit.  But if
the output of the MDCT is more than 16-bit, the encoder will produce bad
output because the exponent will be less than 0, which is not allowed.
Currently I have an av_assert2() there, but maybe that should be changed
to clip the coefficients instead.

Hmmm... you bring up a good point about quality though.  The final
post-rotation does CMUL() which right shifts by 15.  We could possibly
gain some quality by having the post-rotation not right shift and adjust
the coefficient scaling accordingly.

The scaled coefficients still may need to be clipped though.

-Justin