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

Wed Jun 24 07:56:14 CEST 2009

2009/6/24 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Jun 23, 2009 at 01:27:37PM +0700, Vladimir Voroshilov wrote:
>> 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 ?
>
> just write the working formular in the doxy, if it needs some float table add
> that too in the doxy ...

Is attached version ok ?

>
>
> [...]
>> ?g729data.h | ? ?5 +++++
>> ?g729dec.c ?| ? 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> ?2 files changed, 61 insertions(+)
>> 133a11de754c464e24b2f2cef022eeabd850744c ?0001-LSF-decoding-routines.167.patch
>> From 84f17f2ab94b27229767448db0be4980ee299b2a 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 01/25] LSF decoding routines
>
> ok

applied.

>
> [...]
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> When you are offended at any man's fault, turn to yourself and study your
> own failings. Then you will forget your anger. -- Epictetus
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFKQZqpYR7HhwQLD6sRAlmqAJ4noad7JFTp6y9W3+laM6HTiIgo5QCfQh3g
> kF7k8f9LIxD1Q0+lqdDMZRQ=
> =+yg8
> -----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: 0003-doxy-comments-for-LSF-array.169.patch
Type: text/x-patch
Size: 3140 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090624/dc854b4e/attachment.bin>