[FFmpeg-cvslog] r18860 - in trunk/libavcodec: ac3dec.c ac3dec_data.c ac3dec_data.h eac3dec.c

Justin Ruggles justin.ruggles
Wed May 20 04:13:34 CEST 2009


Michael Niedermayer wrote:
> On Tue, May 19, 2009 at 06:23:38PM -0400, Justin Ruggles wrote:
>> Michael Niedermayer wrote:
>>> On Sun, May 17, 2009 at 08:53:27AM +0200, jbr wrote:
>>>> Author: jbr
>>>> Date: Sun May 17 08:53:24 2009
>>>> New Revision: 18860
>>>>
>>>> Log:
>>>> eac3dec: use 16-bit pre-mantissas instead of 24-bit in AHT decoding. it is
>>>> simpler and also fixes a bug in GAQ dequantization.
>>> are you sure 16bit precission is enough for that?
>>> what effect does this compared to double precission floats have on the
>>> PSNR ?
>> stddev:    7.95 PSNR: 78.30 bytes:  3999744/  3999744
> 
> and what is that?
> 
> we have raw -> encoder -> ac3 -> double fp decoder -> raw2
>                               -> 16bit     decoder -> raw3
> 
> PSNR of raw2 - raw vs. PSNR raw3 - raw is what we care about.
> That would make 2 PSNR scores though, a before and a after
> 
> the second part we care about is the difference to a reference decoder
> that should mention which reference decoder is used though

I can't compare to raw in this case because I don't have access to a
professional E-AC-3 encoder which uses the AHT quantization.  But I can
compare to decoded output from Nero's E-AC-3 decoder.

nero vs. ffac3 svn
stddev:  131.16 PSNR: 53.96 bytes:  3998716/  3999744

nero vs. ffac3 w/ doubles for AHT dequant & idct
stddev:  131.30 PSNR: 53.95 bytes:  3998716/  3999744

> 
>> I think 16-bit is enough.
> 
> as a general rule of thumb, calculations in the frequency domain need
> an additional log2(transform length)/2 bits.
> I think the transform size is 512, that means that a minimum of 20 bits
> are needed for all calculations that work on coefficients.
> That is if the output should be 16bits instead of 12bit
> 
> And independant of that decoders must be tested when changes are done
> that lead to different output, that especially true if you throw
> a huge number (8bit) of precission away.
> 
> The idea is not to commit and hope someone notices it.
> 
> and about stddev/PSNR, if you output 16bit and the low 1-3 are just random
> your code is totally broken and should be fixed, removed or replaced.
> Its not that a stddev of 8 is a little off ...
> Thats just an amount at which i wouldnt want to touch the decoder with a
> 10 feet/3 meter pole. We live in a world where people complain about 16bits
> being not enough, choping the precission down to 12 is definitly not ok
> 
> Also to repeat it again, please test, test and test!
> and, maybe the ac3 spec conatins some rules about minimum precission for
> compliant decoders, if not please look at mp3 an use its compliance
> requirement as a minimum.

In the change I made, there are 2 places where precision is lost.

One is in the random mantissas.  In that case, the encoder thought it
was not worth giving those any bits at all, so I don't see this as a
problem.

The other is in the 6-point IDCT, which splits a single "pre-mantissa"
value into 6 mantissas, one for each of the 6 AC-3 blocks.  The result
is shifted to 24-bit before combining with exponents.

The AC-3 spec does not give requirements regarding precision.  The only
clues are in the precision of the tables, and occasionally some vague
wording in the text.  For the IDCT, the spec just says, "Any fast
technique may be used to invert the DCT in Enhanced AC-3 decoders."  The
tables for pre-mantissas, which are the input to the IDCT are all
16-bit.  The output is treated like normal AC-3 mantissas, which are
also only 16-bit.  I didn't see the point of shifting the 16-bit
pre-mantissas up to 24 bits just for the 6-point IDCT.

-Justin



More information about the ffmpeg-cvslog mailing list