[FFmpeg-soc] [soc]: r5570 - in amr: amrnbdata.h amrnbdec.c

[Michael Niedermayer]

On Fri, Jan 08, 2010 at 10:19:54PM -0500, Vitor Sessak wrote:
> cmcq wrote:
>> Author: cmcq
>> Date: Fri Jan  8 18:16:25 2010
>> New Revision: 5570
>> Log:
>> Save about 1KB of space using a field-to-bit lookup instead of 
>> bit-to-field.
>> Modified:
>>    amr/amrnbdata.h
>>    amr/amrnbdec.c
>> Modified: amr/amrnbdata.h
>> ==============================================================================
>> --- amr/amrnbdata.h	Fri Jan  8 18:13:05 2010	(r5569)
>> +++ amr/amrnbdata.h	Fri Jan  8 18:16:25 2010	(r5570)
>> @@ -68,15 +68,6 @@ enum Mode {
>>  #define LP_FILTER_ORDER 10        ///< linear predictive coding filter 
>> order
>
> [...]
>
>> +// Each field in AMRNBFrame is stored as:
>> +//   one byte of 16 * index in AMRNBFrame relative to previous field
>> +//               + number of bits in the field
>> +//   then, one byte for each bit of the field (from most-significant to 
>> least)
>> +//         of the position of that bit in the AMR frame.
>> +static const uint8_t order_MODE_4k75[] = {
>> +    0x28,   7,   6,   5,   4,   3,   2,   1,   0,
>> +    0x18,  15,  14,  13,  12,  11,  10,   9,   8,
>> +    0x17,  51,  35,  34,  50,  33,  49,  32,
>> +    0x38,  23,  22,  21,  20,  19,  18,  43,  42,
>
> [...]
>
>>      if (mode <= MODE_DTX) {
>>          uint16_t *data = (uint16_t *)&p->frame;
>> -        const AMROrder *order = amr_unpacking_bitmaps_per_mode[mode];
>> -        int i;
>> +        const uint8_t *order = amr_unpacking_bitmaps_per_mode[mode];
>> +        int field_header; // 16 * relative field index + number of field 
>> bits
>>           memset(&p->frame, 0, sizeof(AMRNBFrame));
>> -        for (i = 0; i < mode_bits[mode]; i++)
>> -            data[order[i].index] += get_bits1(&gb) << order[i].bit;
>> +        buf++;
>> +        while ((field_header = *order++)) {
>> +            int field = 0;
>> +            data += field_header >> 4;
>> +            field_header &= 0xf;
>> +            while (field_header--) {
>> +               int bit = *order++;
>> +               field <<= 1;
>> +               field |= buf[bit >> 3] >> (bit & 7) & 1;
>> +            }
>> +            *data = field;
>> +        }
>
> I think that this is only guaranteed to work if you declare the AMRNBFrame 
> struct as packed, since the spec allows the compiler to add some padding 
> space inside structs otherwise.

the struct contains only uint16_t fields, so this would seem odd to me if
a compiler did it but iam not saying no compiler does ...


>
> Also I'm undecided if the extra complexity is worth the 1kb smaller tables 
> (of whose only 400 bytes at most would be used in a single frame). Michael, 
> since you are the one who suggested trying to make the tables smaller, was 
> that you had in mind?

I like the new code better than the old

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100109/ff07fe30/attachment.pgp>