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

[Michael Niedermayer]

On Sun, Jan 10, 2010 at 10:05:34PM -0500, Vitor Sessak wrote:
> Vitor Sessak wrote:
>> Vitor Sessak wrote:
>>> Ronald S. Bultje wrote:
>>>> Hi Vitor,
>>>>
>>>> On Jan 7, 2010, at 10:23 PM, Vitor Sessak <vitor1001 at gmail.com> wrote:
>>>>> in:                      { 2, 2.2,  7,  9.5, 10  }
>>>>> out:                     { 2,   4,  7,  9.5, 10  }
>>>>> ff_acelp_reorder_lsf():  { 2,   4,  7,  9.5, 11.5}
>>>>> sane:                    { 2,   4,  7,  9.5, 11.5}
>>>>>
>>>>> here we see a problem
>>>>
>>>> Ah, I see what you mean, min_dist is not enforced for the last lsf. Very 
>>>> interesting. For WMAVoice, that's not the case, it's exactly like the 
>>>> int version. OK, I guess that makes the discussion moot then.
>>>>
>>>> Does output quality audibly differ when adding in this "feature"? (It 
>>>> might just be a decoder bug, WMAVoice is full of them.) Deviating from 
>>>> binary can sometimes be OK...
>>>>
>>>> If the output doesn't change or worsens, patch OK (but please explain 
>>>> this difference in a doxy); if it improves, it might be worth using that 
>>>> instead.
>>>
>>> No, the output gets completely garbled if I do like in 
>>> ff_acelp_reorder_lsf().
>>>
>>> Does anyone else wants to comment/review this codec? If nobody comment in 
>>> a couple of days I'll commit it.
>> Committed.
>> Now the patch that adds 16k support. I'm undecided if I should put this 
>> code in a separated file...
>
> I've just noticed that this decoder depends on a patch that I forgot to 
> attach. It that add floating point versions of some fixed point code 
> already in SVN.
[...]
> Index: libavcodec/acelp_pitch_delay.c
> ===================================================================
> --- libavcodec/acelp_pitch_delay.c	(revision 20587)
> +++ libavcodec/acelp_pitch_delay.c	(working copy)
> @@ -120,6 +120,19 @@
>  #endif
>  }
>  
> +float ff_acelp_decode_gain_codef(float gain_corr_factor, const float *fc_v,
> +                                 float mr_energy, const float *quant_energy,
> +                                 const float *ma_prediction_coeff,
> +                                 int subframe_size, int ma_pred_order)
> +{
> +    mr_energy +=
> +        ff_dot_productf(quant_energy, ma_prediction_coeff, ma_pred_order);
> +
> +    mr_energy = gain_corr_factor * exp(M_LN10 * mr_energy / 20.) /

if written like:
M_LN10 / 20 * mr_energy 

then the compiler can simplify the / out, otherwise it would need
the associative law, which does not hold in a bitexact sense for floats

besides this ive no comment, review of this patch left to our 
*celp coding expert team

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100111/2ad3e2aa/attachment.pgp>