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

Diego Biurrun diego
Sun Jan 3 23:20:48 CET 2010


On Sun, Jan 03, 2010 at 11:16:20PM +0100, Vitor Sessak wrote:
> 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...

It's very easy: Keep the #defines that are only used in the .c file in
the .c file.  Why should they be put anywhere else except to make the
code less local and thus less readable?  Putting them all in the .h file
just because you can gains you nothing.  It will of course still compile
but you need to read more context to understand the code in the .c file.

Diego



More information about the ffmpeg-devel mailing list