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

[Vladimir Voroshilov]

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 ?

> [...]
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFKOoxFYR7HhwQLD6sRAg44AKCElhrqqsS+3Dza3k5Opfs8IutBHwCggEcN
> CMPe/2ba3UT6u60IzPUGJQE=
> =hTxq
> -----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