[FFmpeg-devel] [PATCH] G.729 LSF decoding

Tue Jun 23 08:27:37 CEST 2009

2009/6/22 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Jun 22, 2009 at 05:38:58PM +0700, Vladimir Voroshilov wrote:
>> 2009/6/22 Michael Niedermayer <michaelni at gmx.at>:
>> > On Sat, Jun 20, 2009 at 02:23:07PM +0700, Vladimir Voroshilov wrote:
>> >> >> ?g729data.h | ? 11 +++++++++
>> >> >> ?g729dec.c ?| ? 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >> ?2 files changed, 81 insertions(+)
>> >> >> 180ee12ea9d2d6990454e9bc3a25d29828c2e0b2 ?0001-LSF-decoding-routines.161.patch
>> >> >> From e2e6b6b18255c9d24f9cc0cf60dfbd1c4ccbcef1 Mon Sep 17 00:00:00 2001
>> >> >> From: Vladimir Voroshilov <voroshil at gmail.com>
>> >> >> Date: Sat, 6 Jun 2009 08:00:56 +0700
>> >> >> Subject: [PATCH] LSF decoding routines
>> >> >>
>> >> >>
>> >> >> diff --git ffmpeg-r19218/libavcodec/g729data.h ffmpeg-r19218_v161/libavcodec/g729data.h
>> >> >> index 796d24e..587db67 100644
>> >> >> --- ffmpeg-r19218/libavcodec/g729data.h
>> >> >> +++ ffmpeg-r19218_v161/libavcodec/g729data.h
>> >> >> @@ -262,4 +262,15 @@ static const int16_t cb_ma_predictor[2][MA_NP][10] = { /* (0.15) */
>> >> >> ? ? ?{ 3024, ?1592, ? 940, ?1631, ?1723, ?1579, ?2034, ?2084, ?1913, ?2601}
>> >> >> ? ?}
>> >> >> ?};
>> >> >> +
>> >> >> +/**
>> >> >> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 3
>> >> >> + * ff_g720_cb_ma_predictor_sum[j][i] = ?1 - sum ( ff_g729_cb_ma_predictor[k][i] ), j=0..1, i=0..9
>> >> >> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?k=0
>> >> >> + */
>> >> >
>> >> > this doesnt look correct
>> >>
>> >> It does in floating-point.
>> >
>> > no it does not, this formula is not correct and this is not a matter of
>> > scaling some constant
>> >
>> >
>> >> I've changed "1" to "32765" (now it is correct for fixed-point) and added notice
>> >> why values may slightly differ in array and formula.
>> >
>> > I think ive said it many month ago already but if you cannot write
>> > documentation that matches the code then do NOT write documentation. Wrong
>> > documentation is far worse than no documentation.
>> >
>> > first, this set all entries [j][i] for j=0..1, i=0..9, but j is never
>> > used in the formula, still [0][i] is very different from [1][i], thats not
>> > just sloppy, then you used constants like 1 instead of 32765 thats also not
>> > a little off
>>
>> Yes, i really missed third dimension at right side.
>> With below formula i get about +/-1 differences between formula and
>> table values.
>>
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?3
>> ff_g720_cb_ma_predictor_sum[j][i] = ?32765 - sum (
>> ff_g729_cb_ma_predictor[j][k][i] ), j=0..1, i=0..9
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? k=0
>>
>> > and finally it still doesnt match the table and your solution is just adding
>> > a note saying that initial calculation was done in floating point, what is
>> > that supposed to mean? Is summing 16bit values in float giving differnt
>> > results than integers for you? what kind of cpu is that?
>>
>> I'm afraid you don't understand me.
>> Imagine there is some array ?X[N] (floating-point). S=sum(X[n])
>> and there is another array X_fp[N] (foxed-point) X_fp[n]= X[n] * FP_BASE
>> S_fp=sum(X_fp[n])
>>
>> I just wanted to say that S and S_fp in above sample can slightly differ.
>
> what you wrote in the doxy was
> "Values in array may slightly differ, since initial calculation was done in floating-point."
>
> this is totally backward, you KNOW they differ and i suspect you have no
> faint clue if any calculations where done in floating point by the ITU
> comitee to build these tables.

>
> That would make it a
> "Values do differ slightly, we do not know why"
>
> If OTOH you do know how ITU build these tables then this should be in the
> doxy not a formlar that differs by +-2 or something.

You right, i don't know how did they build this table. But, i can
guess, that they took
floating-point version of MA predictor codebook and build it, using
equation 20 in section 3.2.4 of specification.
For fixed point version they just convert already built table to fixed-point.

To check my guessing, i've written another sample program:
1. got floating-point version of cb_ma_predictor (named "fg" in reference code)
2. got floating-point version of cm_ma_predictor_sum (named "fg_sum"
in reference code)
3. applied floating-point formula to 1 and got exactly 2 (no differences).
4. convert "fg_sum" to fixed-point and got exactly my
cb_ma_predictor_sum (no differences).

How my comment  should look to make above things clear for others ?

Anyway, to avoid blocking this patch due to discussing about comments,
i've splitted it to two peices.

>
> [...]
>
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you think the mosad wants you dead since a long time then you are either
> wrong or dead since a long time.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFKP3FfYR7HhwQLD6sRAgNeAJ9hLJNv5dGjALWv4rlD4H+6BoJMpgCdEr9a
> 8MaCivPtbYgj7lcqnAzNN0g=
> =J2Pi
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-LSF-decoding-routines.167.patch
Type: text/x-patch
Size: 4455 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090623/a53eabf9/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-doxy-comments-for-LSF-array.167.patch
Type: text/x-patch
Size: 2015 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090623/a53eabf9/attachment-0001.bin>