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

[Michael Niedermayer]

On Sat, Jan 09, 2010 at 09:29:47AM -0500, Vitor Sessak wrote:
> 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"?

dunno, benchmark

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/e2a9eb77/attachment.pgp>