[FFmpeg-devel] [PATCH] mips: Optimization of AC3 FP encoder and EAC3 FP decoder

Zivkovic, Bojan (c) bojan at mips.com
Thu Nov 1 15:55:40 CET 2012


Hello Vitor, thanks for the comments.

> > +static void ac3_extract_exponents_mips(uint8_t *exp, int32_t *coef, int nb_coefs)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < nb_coefs; i++) {
> > +        int e;
> > +        int v = abs(coef[i]);
> > +        if (v == 0)
> > +            e = 24;
> > +        else {
> > +            e = 23 - av_log2(v);
> > +            if (e < 0) {
> > +                e = 0;
> > +                coef[i] = av_clip(coef[i], -16777215, 16777215);
> > +            }
> > +        }
> > +        exp[i] = e;
> > +    }
> > +}
> 
> This does a very different thing than the C version of the same 
> function. Why is that good/necessary?
> 
> If you found a bug/improvement, the fix does not belong to MIPS-specific 
> code, it should be fixed in arch-independent code...

Actually, after reviewing the code, we found that optimization of this function
is not necessary because it was developed for an older version of FFmpeg, where
this function was more complex. It will be removed in the next patch.

> > +static void ac3_bit_alloc_calc_bap_mips(int16_t *mask, int16_t *psd,         
> > +                                        int start, int end,                  
> > +                                        int snr_offset, int floor,           
> > +                                        const uint8_t *bap_tab, uint8_t *bap)
> > +{                                                                            
> > +    int bin, band, band_end, address;                                        
> > +    int val, temp1, temp2;                                                   
> > +                                                                             
> > +    /* special case, if snr offset is -960, set all bap's to zero */         
> > +    if (snr_offset == -960) {                                                
> > +        memset(bap, 0, AC3_MAX_COEFS);                                       
> > +        return;                                                              
> > +    }                                                                        
> > +                                                                             
> > +    bin  = start;                                                            
> > +    band = ff_ac3_bin_to_band_tab[start];                                    
> > +    do {                                                                     
> > +        int m = (FFMAX(mask[band] - snr_offset - floor, 0) & 0x1FE0) + floor;
> > +        band_end = ff_ac3_band_start_tab[++band];                            
> > +        band_end = FFMIN(band_end, end);                                     
> > +                                                                             
> > +        for (; bin < band_end; bin++) {                                      
> > +            val = (psd[bin] - m) >> 5;                                       
> > +            __asm__ volatile (                                               
> > +                "sra    %[temp1],   %[val],     31          \n\t"            
> > +                "xor    %[address], %[temp1],   %[val]      \n\t"            
> > +                "addiu  %[temp1],   %[val],     -63         \n\t"            
> > +                "sra    %[temp2],   %[temp1],   31          \n\t"            
> > +                "xor    %[temp1],   %[temp1],   %[temp2]    \n\t"            
> > +                "subu   %[address], %[address], %[temp1]    \n\t"            
> > +                "addiu  %[address], %[address], 63          \n\t"            
> > +                "sra    %[address], %[address], 1           \n\t"            
> > +                : [temp1] "=&r" (temp1), [temp2] "=&r" (temp2),              
> > +                  [address] "=&r" (address)                                  
> > +                : [val] "r" (val)                                            
> > +            );                                                               
> > +            bap[bin] = bap_tab[address];                                     
> > +        }                                                                    
> > +    } while (end > band_end);                                                
> > +}                                                                            
>                                                                                 
> Hmm, does using the same trick in plain C produces worse code?           

It depends on the compiler, but it should not produce worse code.

But, after additional analysis of the code, we found a better solution for doing
this in assembly, so we will provide that in the next patch.

-Bojan


More information about the ffmpeg-devel mailing list