[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes

[Vitor Sessak]

Michael Niedermayer wrote:
> On Mon, Dec 28, 2009 at 11:32:56PM +0100, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Mon, Dec 21, 2009 at 03:43:02PM +0100, Vitor Sessak wrote:
>>>   
>>>> Michael Niedermayer wrote:
>>>>     
>>>>> On Sat, Dec 19, 2009 at 03:17:37PM +0100, Vitor Sessak wrote:
>>>>>       
>>>>>> Michael Niedermayer wrote:
>>>>>>         
>>>>>>> On Fri, Dec 18, 2009 at 06:07:45PM +0100, Vitor Sessak wrote:
>>>>>>>           
>>>>>>>> Diego Biurrun wrote:
>>>>>>>>             
>>>>>>>>> On Fri, Dec 18, 2009 at 05:52:13PM +0100, Vitor Sessak wrote:

[...]

>> done as suggested.
>>
>>> btw what value does isp[LP_FILTER_ORDER - 1] have?
>>> its not 1 or -1 or something trivial ?
>>>   
>> no :(
>>
>> New patch attached, together with Diego's cosmetics.
> [...]
>> +#define L_INTERPOL       11
>> +
>> +#define LSFQ_DIFF_MIN    50.0
>> +
>> +#define LP_FILTER_ORDER  10
>> +#define L_SUBFR_SIPR     48
> 
> these could benefit from doxy
> 
> 
> [...]
>> +typedef struct {
>> +    AVCodecContext *avctx;
>> +    DSPContext dsp;
>> +
>> +    SiprModeParam m;
>> +    SiprMode mode;
>> +
> 
>> +    float past_gain_pit;
> 
> pit? pitch? 
> 
> 
>> +    float lsf_hist[LP_FILTER_ORDER];
>> +
>> +    float excitation[L_INTERPOL + PITCH_DELAY_MAX + 5*L_SUBFR_SIPR];
>> +
>> +    DECLARE_ALIGNED_16(float, synth_buf[LP_FILTER_ORDER + 5*L_SUBFR_SIPR + 6]);
>> +
>> +    float lsp_history[16];
> 
> you mix _history and _hist, please dont do this
> 
> 
>> +    float gain_mem;
>> +    float energy_history[4];
> 
>> +    float ymem[2];
> 
> some variables would benefit from doxy, this is one of them

all above done.

> ive no further comments, remaining review left to our celp experts

Do you have anyone in mind in particular? I can think of:

- Robert, who wrote most of the SoC AMR decoder
- Ronald, who is working on a WMAVoice decoder
- Benjamin, who mentored AMR+QCELP
- Reynaldo, who wrote the QCELP decoder
- Collin, who wrote the rest of the AMR decoder
- Voroshil, who wrote a G729 decoder and RE'd most of SIPR but I'm not 
sure if he is following the work I'm doing now

I think Robert could give quite a good help, since SIPR is so similar to 
AMR-NB...

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add_sipr2_5.diff
Type: text/x-patch
Size: 39793 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091231/0457b789/attachment.bin>