[libav-devel] [PATCH] imc: some cosmetics

Luca Barbato lu_zero at gentoo.org
Wed May 23 19:35:20 CEST 2012


On 23/05/12 09:59, Kostya Shishkov wrote:
> On Tue, May 22, 2012 at 08:37:27PM +0200, Diego Biurrun wrote:
>> On Tue, May 22, 2012 at 07:46:41PM +0200, Kostya Shishkov wrote:
>>> ---
>>> I plan to reuse this code for Intel Audio Coder, so I thought it could do with
>>> some diegoing.
>>
>> :)
>>
>>> --- a/libavcodec/imc.c
>>> +++ b/libavcodec/imc.c
>>> @@ -146,13 +146,13 @@ static av_cold int imc_decode_init(AVCodecContext * avctx)
>>>  
>>>      /* Generate a square root table */
>>>  
>>> -    for(i = 0; i < 30; i++) {
>>> +    for (i = 0; i < 30; i++) {
>>>          q->sqrt_tab[i] = sqrt(i);
>>>      }
>>
>> I'd drop the parentheses here, like in most other one-line for statements.
> 
> I hope you meant braces. 
> 
>>> @@ -176,8 +176,9 @@ static av_cold int imc_decode_init(AVCodecContext * avctx)
>>>  
>>> +static void imc_calculate_coeffs(IMCContext* q, float* flcoeffs1,
>>
>> *q - place the star with the variable name, not the type name.
>> There's a bunch more instances below...
> 
> done
> 
>>> @@ -200,29 +201,29 @@ static void imc_calculate_coeffs(IMCContext* q, float* flcoeffs1, float* flcoeff
>>>          workT2[cnt2-1] = workT2[cnt2-1] + workT3[i];
>>
>> spaces around -
>>
>>> -    for(i = 1; i < BANDS; i++) {
>>> +    for (i = 1; i < BANDS; i++) {
>>>          accum = (workT2[i-1] + accum) * imc_weights1[i-1];
>>
>> ditto
>>
>>>      float tmp, tmp2;
>>>      //maybe some frequency division thingy
>>
>> space after //
>>
>>> @@ -290,15 +293,17 @@ static void imc_decode_level_coefficients(IMCContext* q, int* levlCoeffBuf, floa
>>> +    //FIXME maybe flag_buf = noise coding and flcoeffs1 = new scale factors
>>> +    //      and flcoeffs2 old scale factors
>>> +    //      might be incomplete due to a missing table that is in the binary code
>>
>> ditto and /* */ would be nicer here IMO
>>
>>> @@ -327,31 +334,31 @@ static int bit_allocation (IMCContext* q, int stream_format_code, int freebits,
>>> -        if (((band_tab[i+1] - band_tab[i])/2) >= q->bandWidthT[i])
>>> +        if (((band_tab[i + 1] - band_tab[i])/2) >= q->bandWidthT[i])
>>
>> spaces around /
>>
>>> -        q->flcoeffs4[i] = q->flcoeffs4[i] + xTab[(indx*2 + (q->flcoeffs1[i] < highest)) * 2 + flag];
>>> +        q->flcoeffs4[i] += xTab[(indx*2 + (q->flcoeffs1[i] < highest)) * 2 + flag];
>>
>> spaces around *
>>
>>> @@ -361,22 +368,22 @@ static int bit_allocation (IMCContext* q, int stream_format_code, int freebits,
>>>  
>>> -    for(i = 0; i < BANDS/2; i++) {
>>> +    for (i = 0; i < BANDS/2; i++) {
>>
>> spaces around /
>>
>>>          rres = summer - freebits;
>>> -        if((rres >= -8) && (rres <= 8)) break;
>>> +        if ((rres >= -8) && (rres <= 8)) break;
>>
>> Break the line.
>>
>>>          summer = 0;
>>>          iacc = 0;
>>
>> Align.
>>
>>> @@ -446,23 +455,23 @@ static int bit_allocation (IMCContext* q, int stream_format_code, int freebits,
>>> -            //if(lowest >= 1.e10) break;
>>> +            //if (lowest >= 1.e10) break;
>>
>> space after // and break the line
>>
>>> @@ -472,49 +481,50 @@ static int bit_allocation (IMCContext* q, int stream_format_code, int freebits,
>>>  
>>> -            for(j = band_tab[i]; j < (band_tab[i+1]-1); j += 2) {
>>> -                if(!get_bits1(&q->gb)){//0
>>> +            for (j = band_tab[i]; j < (band_tab[i + 1]-1); j += 2) {
>>> +                if (!get_bits1(&q->gb)) {//0
>>
>> spaces around - and //
>>
>>> +                    if (get_bits1(&q->gb)) {//11
>>
>> same
>>
>>> +                        q->skipFlagBits[i] += 3;
>>> +                        q->skipFlags[j + 1] = 0;
>>> +                        if (!get_bits1(&q->gb)) {//100
>>
>> see above
>>
>>> +                            q->skipFlags[j] = 1;
>>>                              q->skipFlagCount[i]++;
>>> -                        }else{//101
>>> +                        } else {//101
>>
>> again
>>
>>>                              q->skipFlags[j]=0;
>>
>> spaces around =
>>
>>> @@ -565,16 +577,17 @@ static void imc_adjust_bit_allocation (IMCContext* q, int summer) {
>>>  
>>>      /* prerotation */
>>> -    for(i=0; i < COEFFS/2; i++){
>>> +    for (i=0; i < COEFFS/2; i++) {
>>
>> spaces around /
>>
>>> @@ -582,44 +595,47 @@ static void imc_imdct256(IMCContext *q) {
>>>      q->fft.fft_calc   (&q->fft, q->samples);
>>
>> This spacing looks weird.
> 
> not at all:
>     q->fft.fft_permute(&q->fft, q->samples);
>     q->fft.fft_calc   (&q->fft, q->samples);
> 
> [...]
> 
> The rest of issues should be addressed.

Should be good to go.

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero



More information about the libav-devel mailing list