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

[Michael Niedermayer]

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

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?


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- 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/20090622/5da2e4b1/attachment.pgp>