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

[Vitor Sessak]

Michael Niedermayer wrote:
> 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 ...

Is there any disadvantage in using "packed"?

-Vitor