[FFmpeg-devel] [PATCH] G.729 LSF decoding
Michael Niedermayer
michaelni
Fri Jun 19 19:16:36 CEST 2009
On Fri, Jun 19, 2009 at 09:11:58AM +0700, Vladimir Voroshilov wrote:
> 2009/6/19 Michael Niedermayer <michaelni at gmx.at>:
> > On Fri, Jun 19, 2009 at 01:58:29AM +0700, Vladimir Voroshilov wrote:
> >> 2009/6/19 Michael Niedermayer <michaelni at gmx.at>:
> >> > On Thu, Jun 18, 2009 at 11:18:53PM +0700, Vladimir Voroshilov wrote:
> >> >> 2009/6/18 Michael Niedermayer <michaelni at gmx.at>:
> >> >> > On Thu, Jun 18, 2009 at 08:26:56AM +0700, Vladimir Voroshilov wrote:
> >> >> >> 2009/6/18 Michael Niedermayer <michaelni at gmx.at>:
> >> >> >> > On Thu, Jun 18, 2009 at 02:10:52AM +0700, Vladimir Voroshilov wrote:
> >> >> >> >> 2009/6/18 Diego Biurrun <diego at biurrun.de>:
> >> >> >> >> > On Thu, Jun 18, 2009 at 02:00:37AM +0700, Vladimir Voroshilov wrote:
> >> >> >> >> >> First patch for main decoding routine.
> >> >> >> >> >> Decodes LSF (Linear Spectrum Frequencies, if i'm not wrong).
> >> >> >> >> >>
> >> >> >> >> >> --- ffmpeg-r19188/libavcodec/g729dec.c
> >> >> >> >> >> +++ ffmpeg-r19188_v153/libavcodec/g729dec.c
> >> >> >> >> >> @@ -114,8 +119,74 @@ static inline int get_parity(uint8_t value)
> >> >> >> >> >>
> >> >> >> >> >> + ? ?/* Rotate past_quantizer_outputs. */
> >> >> >> >> >> + ? ?for(k=MA_NP-1; k>0; k--)
> >> >> >> >> >
> >> >> >> >> > nit: inconsistently formatted for construct
> >> >> >> >>
> >> >> >> >> Damn. One was overlooked...
> >> >> >> >> Fixed.
> >> >> >> >>
> >> >> >> >
> >> >> >> > [...]
> >> >> >> >> @@ -114,8 +119,74 @@ static inline int get_parity(uint8_t value)
> >> >> >> >> ? ? return (0x6996966996696996ULL >> (value >> 2)) & 1;
> >> >> >> >> ?}
> >> >> >> >>
> >> >> >> >> +/**
> >> >> >> >> + * Saves quantized LSF coefficients for use in next frame.
> >> >> >> >> + * @param past_quantizer_outputs [in/out] (2.13) quantizer outputs from previous frames
> >> >> >> >> + * @param quantizer_output (2.13) current quantizer output
> >> >> >> >> + */
> >> >> >> >> +static void lq_rotate(int16_t past_quantizer_outputs[MA_NP][10],
> >> >> >> >> + ? ? ? ? ? ? ? ? ? ? ?const int16_t* quantizer_output)
> >> >> >> >> +{
> >> >> >> >> + ? ?int k;
> >> >> >> >> +
> >> >> >> >> + ? ?/* Rotate past_quantizer_outputs. */
> >> >> >> >> + ? ?for (k = MA_NP - 1; k > 0; k--)
> >> >> >> >> + ? ? ? ?memcpy(past_quantizer_outputs[k], past_quantizer_outputs[k-1], 10 * sizeof(int16_t));
> >> >> >> >> + ? ?memcpy(past_quantizer_outputs[0], quantizer_output, 10 * sizeof(int16_t));
> >> >> >> >> +}
> >> >> >> >
> >> >> >> > used just once and 3 lines, must be inlined
> >> >> >> >
> >> >> >> > note: memcpy() will not be approved before the neccessarity is clear
> >> >> >>
> >> >> >> Replaced several memcpy with pointer rotation via memmove.
> >> >> >> Is such fix enough (patch attached)?
> >> >> >
> >> >> > its better, still the 2 line function that is used once remains and
> >> >> > as said it should be inlined
> >> >>
> >> >> I misunderstood you about inlining. Now fixed.
> >> >>
> >> >> >
> >> >> > [...]
> >> >> >> @@ -124,6 +192,23 @@ static av_cold int decoder_init(AVCodecContext * avctx)
> >> >> >> ? ? ?/* Both 8kbit/s and 6.4kbit/s modes uses two subframes per frame. */
> >> >> >> ? ? ?avctx->frame_size = SUBFRAME_SIZE << 1;
> >> >> >>
> >> >> >> + ? ?for (k = 0; k < MA_NP + 1; k++) {
> >> >> >
> >> >> >> + ? ? ? ?ctx->past_quantizer_outputs[k] = av_malloc(10 * sizeof(int16_t));
> >> >> >
> >> >> > this is unacceptable
> >> >> > put the array in the struct, not allocate 20byte pieces with malloc
> >> >>
> >> >> Is attached acceptable?
> >> >> Or can it be improved ?somehow?
> >> >>
> >> >>
> >> >> >
> >> >> > [...]
> >> >> > --
> >> >> > Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >> >> >
> >> >> > In fact, the RIAA has been known to suggest that students drop out
> >> >> > of college or go to community college in order to be able to afford
> >> >> > settlements. -- The RIAA
> >> >> >
> >> >> > -----BEGIN PGP SIGNATURE-----
> >> >> > Version: GnuPG v1.4.9 (GNU/Linux)
> >> >> >
> >> >> > iD8DBQFKOhupYR7HhwQLD6sRAirVAJ416btGbzTZMdk+piKNO8Li7QR2iwCgidy9
> >> >> > 8I0/9076Por+onuUpqMksWA=
> >> >> > =90Gn
> >> >> > -----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
> >> >
> >> >> ?g729data.h | ? ?9 +++++++
> >> >> ?g729dec.c ?| ? 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >> ?2 files changed, 79 insertions(+)
> >> >> 92e00ddb456d759a865f027baa9472f52189ff32 ?0002-LSF-decoding-routines.159.patch
> >> >> From 68af3a99f7f09671070c8f1cfe2414dead6f1216 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-r19215/libavcodec/g729data.h ffmpeg-r19215_v159/libavcodec/g729data.h
> >> >> index 796d24e..ce6ab3d 100644
> >> >> --- ffmpeg-r19215/libavcodec/g729data.h
> >> >> +++ ffmpeg-r19215_v159/libavcodec/g729data.h
> >> >> @@ -262,4 +262,13 @@ static const int16_t cb_ma_predictor[2][MA_NP][10] = { /* (0.15) */
> >> >> ? ? ?{ 3024, ?1592, ? 940, ?1631, ?1723, ?1579, ?2034, ?2084, ?1913, ?2601}
> >> >> ? ?}
> >> >> ?};
> >> >> +
> >> >> +/**
> >> >> + * ff_g729_cb_ma_predictor_sum[i] := 1-sum{1}{4}{ff_g729_cb_ma_predictor[k][i]}
> >> >
> >> > what is := supposed to mean? its not math and not C, so it seems not ideal
> >> > in a C file ...
> >>
> >>
> >> Entire equation is equall to:
> >>
> >> for (i=0; i<10; i++ {
> >> ?ff_g729_cb_ma_predictor_sum[i] = 1;
> >> ?for (k=0; k<3; k++)
> >> ? ?ff_g729_cb_ma_predictor_sum[i] -= ff_g729_cb_ma_predictor[k][i]
> >> }
> >>
> >> Should i replace that short equation with this C code ?
> >
> > no, i suggest ascii art with the proper math symbols
>
> Rewrote in math art.
> I left "sum", since upper sigma takes too lot of space.
>
>
>
>
> --
> Regards,
> Vladimir Voroshilov mailto:voroshil at gmail.com
> JID: voroshil at gmail.com, voroshil at jabber.ru
> ICQ: 95587719
> 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
[...]
> @@ -114,8 +120,60 @@ static inline int get_parity(uint8_t value)
> return (0x6996966996696996ULL >> (value >> 2)) & 1;
> }
>
> +/**
> + * Decodes LSP coefficients from L0-L3 (3.2.4).
> + * @param lsfq [out] (2.13) quantized LSF coefficients
> + * @param past_quantizer_outputs [in/out] (2.13) quantizer outputs from previous frames
> + * @param ma_predictor switched MA predictor of LSP quantizer
> + * @param vq_1st first stage vector of quantizer
> + * @param vq_2nd_low second stage lower vector of LSP quantizer
> + * @param vq_2nd_high second stage higher vector of LSP quantizer
> + */
> +static void lsf_decode(int16_t* lsfq, int16_t* past_quantizer_outputs[MA_NP + 1],
> + int16_t ma_predictor,
> + int16_t vq_1st, int16_t vq_2nd_low, int16_t vq_2nd_high)
> +{
> + int i,j,k;
> + static const uint8_t min_distance[2]={10, 5}; //(2.13)
> + int16_t* quantizer_output = past_quantizer_outputs[MA_NP];
> + int16_t diff; //(2.13)
> + int sum; //(2.28)
> +
> + for (i = 0; i < 5; i++) {
> + quantizer_output[i] = cb_lsp_1st[vq_1st][i ] + cb_lsp_2nd[vq_2nd_low ][i ];
> + quantizer_output[i + 5] = cb_lsp_1st[vq_1st][i + 5] + cb_lsp_2nd[vq_2nd_high][i + 5];
> + }
> +
> + for (j = 0; j < 2; j++) {
> + for (i = 1; i < 10; i++) {
> + diff = (quantizer_output[i - 1] - quantizer_output[i] + min_distance[j]) >> 1;
declaration and init can be merged also it does ot need exactly 16 bit
> + if (diff > 0) {
> + quantizer_output[i - 1] -= diff;
> + quantizer_output[i ] += diff;
> + }
> + }
> + }
> +
> + for (i = 0; i < 10; i++) {
> + sum = quantizer_output[i] * cb_ma_predictor_sum[ma_predictor][i];
same
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20090619/37cd27f0/attachment.pgp>
More information about the ffmpeg-devel
mailing list