[FFmpeg-devel] Review request - ra288.{c,h} ra144.{c,h}

Vitor Sessak vitor1001
Sat Sep 6 13:49:51 CEST 2008


Benoit Fouet wrote:
> Hi,
> 
> Vitor Sessak wrote:
>> Vitor Sessak wrote:
>>   
>>> Hi,
>>>
>>> Those four files never passed a review. I've just finished cleaning them 
>>> up, so if anyone wants to review them (Michael already said he will), 
>>> now is time.
>>>     
>> I think now they can go through another review cycle.
>>
>>   
> 
> [from ra144.c]
>> static void eval_coefs(int *coefs, const int *refl)
>> {
>>     int buffer[10];
>>     int *b1 = buffer;
>>     int *b2 = coefs;
>>     int i, j;
>>
>>     for (i=0; i < 10; i++) {
>>         b1[i] = refl[i] << 4;
>>
>>         for (j=0; j < i; j++)
>>             b1[j] = ((refl[i] * b2[i-j-1]) >> 12) + b2[j];
>>
> 
> seems that will overwrite b1[i] affectation when j == 0

Could you elaborate? For me, when j == 0

b1[0] = ((refl[i] * b2[i-1]) >> 12) + b2[0];

(and this will only be done when i != 0)

>> static void int_to_int16(int16_t *out, const int *inp)
>> {
>>     int i;
>>
>>     for (i=0; i < 30; i++)
>>         *(out++) = *(inp++);
>>
> 
> unneeded parentheses
> 
>> static int eval_refl(int *refl, const int16_t *coefs, RA144Context *ractx)
>> {
>>     int b, i, j;
>>     int buffer1[10];
>>     int buffer2[10];
>>     int *bp1 = buffer1;
>>     int *bp2 = buffer2;
>>
>>     for (i=0; i < 10; i++)
>>         buffer2[i] = coefs[i];
>>
>>     refl[9] = bp2[9];
>>
>>     if ((unsigned) bp2[9] + 0x1000 > 0x1fff) {
>>         av_log(ractx, AV_LOG_ERROR, "Overflow. Broken sample?\n");
>>         return 1;
>>     }
>>
>>     for (i=8; i >= 0; i--) {
>>         b = 0x1000-((bp2[i+1] * bp2[i+1]) >> 12);
>>
>>         if (!b)
>>             b = -2;
>>
>>         for (j=0; j <= i; j++)
>>             bp1[j] = ((bp2[j] - ((refl[i+1] * bp2[i-j]) >> 12)) *
> (0x1000000 / b)) >> 12;
>>         refl[i] = bp1[i];
>>
>>         if ((unsigned) bp1[i] + 0x1000 > 0x1fff)
>>             return 1;
>>
> 
> maybe the test could be done before the refl[i] = bp1[i]

The rest is changed. Thanks for having a look at this.

-Vitor




More information about the ffmpeg-devel mailing list