[Ffmpeg-devel] ac3 encoder volume correction

[Michael Niedermayer]

Hi

On Mon, Nov 20, 2006 at 09:20:48PM -0800, Bill O'Shaughnessy wrote:
> Hello all,
>  after encoding a normalized wave file, ac3enc always encoded it a half
> the volume of the original wave.  After some experimentation I found
> that Justing Ruggles fix did not go far enough.
> I also fixed some multiplies that should be 32 bit multiplies before
> being shifted right 15 bits.
> 
> With this fix ffmpeg encodes an ac3 file to identical volume of the
> input source .wav file.
> 
> Good Luck,
>  Bill O.
> 
> Here it is:
> --- ac3enc.corg    2006-11-20 20:31:04.500000000 -0800
> +++ ac3enc.c    2006-11-20 20:46:52.125000000 -0800
> @@ -330,11 +330,12 @@
> }
> 
> #define MUL16(a,b) ((a) * (b))
> +#define MUL32(a,b) (int32_t)((int32_t)(a) * (int32_t)(b))
> 
> #define CMUL(pre, pim, are, aim, bre, bim) \
> {\
> -   pre = (MUL16(are, bre) - MUL16(aim, bim)) >> 15;\
> -   pim = (MUL16(are, bim) + MUL16(bre, aim)) >> 15;\
> +   pre = (MUL32(are, bre) - MUL32(aim, bim)) >> 15;\
> +   pim = (MUL32(are, bim) + MUL32(bre, aim)) >> 15;\
> }

rejected this change makes no sense
also its not indented by 4 spaces


> 
> 
> @@ -1286,10 +1287,14 @@
> static void lshift_tab(int16_t *tab, int n, int lshift)
> {
>     int i;
> +    int32_t it;

wrong indention, also int32_t is unaccpetable unless required which is not
the case here


> 
>     if (lshift > 0) {
>         for(i=0;i<n;i++) {
> -            tab[i] <<= lshift;
> +            it = ((int32_t)tab[i]) << lshift;
> +        if( it > 32767 ) it = 32767;
> +        if( it < -32767 ) it = -32767;
> +        tab[i] = it;

this is just wrong
the code is
            /* Normalize the samples to use the maximum available
               precision */
            v = 14 - log2_tab(input_samples, N);
            if (v < 0)
                v = 0;
            exp_samples[i][ch] = v - 9;
            lshift_tab(input_samples, N, v);


this by definition scales to the max so cliping per definition is not
needed


>         }
>     } else if (lshift < 0) {
>         lshift = -lshift;
> @@ -1368,9 +1373,9 @@
> 
>             /* apply the MDCT window */
>             for(j=0;j<N/2;j++) {
> -                input_samples[j] = MUL16(input_samples[j],
> +                input_samples[j] = MUL32(input_samples[j],
>                                          ac3_window[j]) >> 15;
> -                input_samples[N-j-1] = MUL16(input_samples[N-j-1],
> +                input_samples[N-j-1] = MUL32(input_samples[N-j-1],
>                                              ac3_window[j]) >> 15;

more nonsense


>             }
> 
> @@ -1379,7 +1384,7 @@
>             v = 14 - log2_tab(input_samples, N);
>             if (v < 0)
>                 v = 0;
> -            exp_samples[i][ch] = v - 9;
> +            exp_samples[i][ch] = v - 10;

this may or may not be correct (=requires quotation from the ac3 spec
saying that this is correct)

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

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is