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

Ronald S. Bultje rsbultje
Thu Jan 7 18:10:16 CET 2010


Hi Vitor,

On Wed, Jan 6, 2010 at 7:47 PM, Vitor Sessak <vitor1001 at gmail.com> wrote:
> Ronald S. Bultje wrote:
>> +static void lsf_decode_fp(float *lsfnew, float *lsf_history,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?const SiprParameters *parm)
>> [..]
>> + ? ?ff_set_min_dist_lsf(lsfnew, LSFQ_DIFF_MIN / 4000., LP_FILTER_ORDER -
>> 1);
>>
>> That value is only used once, you could just adjust the DIFF_MIN directly.
>>
>> + ? ?lsfnew[9] = FFMIN(lsfnew[LP_FILTER_ORDER - 1], 1.3);
>>
>> Aha, so there is a max. value for the last one. I thought so. :-). OK,
>> so here's my suggestion: let's merge this into the ff_set_min_dist()
>> function. WMAVoice same, and so do others.
>>
>
> I'm not really happy about this for two reasons:
>
> 1- There is no max value for AMR and SIPR 16k
> 2- Such a function would work in a slightly different way of what would
> be logical (it doesn't impose a max distance for the last value):
[..]

I'm thinking of a float version of ff_acelp_reorder_lsf() in
libavcodec/lsp.c, I think it is 100% analogous to this code and
WMAVoice uses it too.

> float in = {1, 5, 6, 7, 12}
>
> ff_set_min_dist_lsf_with_max(in, 2.0, 5, 14.0 /* max */);
>
> result: 1, 3, 6, 8, 12

min_dist, not max_dist, so I think the result would be:

result: 1, 5, 7, 9, 12.

right?

(Will look briefly through the patch, but all other comments seem
taken care of so I won't have much more except this.)

Ronald



More information about the ffmpeg-devel mailing list