[FFmpeg-devel] [PATCH] Add AMR-NB decoder, next try

Vitor Sessak vitor1001
Sun Jan 3 23:16:20 CET 2010


Diego Biurrun wrote:
> On Sun, Jan 03, 2010 at 07:29:39PM +0100, Vitor Sessak wrote:
>> Diego Biurrun wrote:
>>> On Sun, Jan 03, 2010 at 06:01:49PM +0100, Vitor Sessak wrote:
>>>> Hi, the SoC AMR decoder is in a pretty nice shape and have already 
>>>> gone  through a few review rounds, so I'll give my try in getting it 
>>>> committed.
>>>>
>>>> --- libavcodec/amrnbdata.h	(revision 0)
>>>> +++ libavcodec/amrnbdata.h	(revision 0)
>>>> +/** Number of impulse response coefficients used for tilt factor */
>>>> +#define AMR_TILT_RESPONSE   22
>>>> +/** Tilt factor = 1st reflection coefficient * gamma_t */
>>>> +#define AMR_TILT_GAMMA_T   0.8
>>>> +/** Adaptive gain control factor used in post-filter */
>>>> +#define AMR_AGC_ALPHA      0.9
>>> The bottom of a *huge* header file with tables does not seem like
>>> a good place for these..
>> They are there because the header is nicely separated in different  
>> sections. A possible solution would be moving the defines to the C file,  
>> but I'm undecided.
> 
> If they are only used in the C file, then they do not belong in the
> header.

This header is not supposed to be included by any other C file, it 
contains static tables (including it anywhere else would needlessly 
duplicate all of them). The problem is that some defines are needed for 
defining the tables (code like "static const int16_t 
lsp_avg_init[LP_FILTER_ORDER] = "). So we either have all the defines in 
the .h or half in the C file half in the .h...

-Vitor



More information about the ffmpeg-devel mailing list