[FFmpeg-devel] [PATCH] Create ff_amr_bit_reorder() in a shared amr.h file

Vitor Sessak vitor1001
Wed Sep 8 22:30:16 CEST 2010

On 09/08/2010 06:51 PM, Marcelo Galv?o P?voa wrote:
> On 8 September 2010 09:07, Vitor Sessak<vitor1001 at gmail.com>  wrote:
>> On 09/07/2010 12:02 AM, Marcelo Galv?o P?voa wrote:
>>> Hello,
>>> I am not sure if it is the best way to do it, maybe it's better to
>>> move the memset outside the function. Unfortunately, I had to convert
>>> several tables in amrnbdata.h from uint8_t to uint16_t because
>>> amrwbdata.h uses the latter.
>> If you split the order tables in two (one for the pulses and one for the
>> rest) can you replace in AMRWB
>> ff_amr_bit_reorder(ctx, size, data, order_mode);
>> by
>> ff_amr_bit_reorder(ctx, size, data, order_mode);
>> ff_amr_bit_reorder(ctx, size2, data + 8, order_mode_pulses);
>> in a way that you can keep uint8_t for the order tables?
> I don't know if I understand that correctly but the pulse bits aren't
> all contained after the first byte. For example, for this pulse:
> 11, AMR_OF(1, pul_ih[2]), 106, 134, 125, 154, 205, 267, 306, 220,
>                            185, 330, 297,
> Some bits would have to be in one table and the rest in another, which
> would break the reordering function.

For this example, you could send data+24 to ff_amr_reorder(), so you 
would subtract all the terms by 24*8 == 96, and it would fit in 8 bits.

But I just found another example:

     11, AMR_OF(1, pul_il[0]), 266, 186, 377, 439, 279, 227, 256, 376,
                               401, 417, 450,

since 450-186 == 264, this field could never fit in 8 bits, no matter 
what one do.

So we have the following options:

1) Convert amrnbdata.h order mode tables to 16-bit like in your patch 
(wastes space)
2) duplicate this code in amrwbdec.c and amrwbdec.c (ugly)
3) Use a preprocessor trick to create both functions (messy).

For the last option, one would move all the code to amr.h (*not* amr.c) 
and put something like in it

#define TABLE_TYPE uint16_t
#define TABLE_TYPE uint8_t

static inline void ff_amr_bit_reorder(uint16_t *out, int size,
                                       const uint8_t *data,
                                       const TABLE_TYPE *ord_table)

So the different files might do either

#include "amr.h"


#include "amr.h"

I prefer slightly option (3), but if others have a strong opinion for 
another option I will not oppose. Michael, Rob, Ronald?


More information about the ffmpeg-devel mailing list