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

[Michael Niedermayer]

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

[...]
> @@ -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

[...]
-- 
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
-------------- 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/20090618/7eb7500c/attachment.pgp>