[FFmpeg-devel] [PATCH] G.729 Frame erasure support for LSF decoding

[Michael Niedermayer]

On Thu, Jun 25, 2009 at 06:20:53PM +0700, Vladimir Voroshilov wrote:
> 2009/6/25 Michael Niedermayer <michaelni at gmx.at>:
[...]
> >> diff --git ffmpeg-r19260/libavcodec/g729data.h ffmpeg-r19260_v171/libavcodec/g729data.h
> >> index 1d64553..02d0b93 100644
> >> --- ffmpeg-r19260/libavcodec/g729data.h
> >> +++ ffmpeg-r19260_v171/libavcodec/g729data.h
> >> @@ -268,6 +268,11 @@ static const int16_t cb_ma_predictor_sum[2][10] = { /* (0.15) */
> >> ? ?{14585, 18333, 19772, 17344, 16426, 16459, 15155, 15220, 16043, 15708}
> >> ?};
> >>
> >> +static const int16_t cb_ma_predictor_sum_inv[2][10] = { /* (3.12) */
> >> + ?{17210, 15888, 16357, 16183, 16516, 15833, 15888, 15421, 14840, 15597},
> >> + ?{ 9202, ?7320, ?6788, ?7738, ?8170, ?8154, ?8856, ?8818, ?8366, ?8544}
> >> +};
> >> +
> >> ?/**
> >> ? * initial LSP coefficients belongs to virtual frame preceding ?the
> >> ? * first frame of the stream
> >
> >> diff --git ffmpeg-r19260/libavcodec/g729dec.c ffmpeg-r19260_v171/libavcodec/g729dec.c
> >> index 628aee0..baf8a08 100644
> >> --- ffmpeg-r19260/libavcodec/g729dec.c
> >> +++ ffmpeg-r19260_v171/libavcodec/g729dec.c
> >> @@ -88,6 +88,8 @@ typedef struct {
> >> ? ? ?int16_t lsfq[10]; ? ? ? ? ? ///< (2.13) quantized LSF coefficients from previous frame
> >> ? ? ?int16_t lsp_buf[2][10]; ? ? ///< (0.15) LSP coefficients (previous and current frames) (3.2.5)
> >> ? ? ?int16_t *lsp[2]; ? ? ? ? ? ?///< pointers to lsp_buf
> >> +
> >> + ? ?int ma_predictor; ? ? ? ? ? ///< switched MA predictor of LSP quantizer
> >> ?} ?G729Context;
> >>
> >> ?static const G729FormatDescription format_g729_8k = {
> >
> > this change looks unneeded
> 
> It does.
> 
> I have to keep MA predictor from last good frame.
> It is used for past quantizer output computation.

if it is used and represents the previous, prev or previous should be in its
name


> 
> >
> >
> >> @@ -155,13 +157,26 @@ static void lsf_decode(int16_t* lsfq, int16_t* past_quantizer_outputs[MA_NP + 1]
> >> ? ? ? ? ?lsfq[i] = sum >> 15;
> >> ? ? ?}
> >>
> >> - ? ?/* Rotate past_quantizer_outputs. */
> >> - ? ?memmove(past_quantizer_outputs + 1, past_quantizer_outputs, MA_NP * sizeof(int16_t*));
> >> - ? ?past_quantizer_outputs[0] = quantizer_output;
> >> -
> >> ? ? ?ff_acelp_reorder_lsf(lsfq, LSFQ_DIFF_MIN, LSFQ_MIN, LSFQ_MAX, 10);
> >> ?}
> >>

> >> +static void lsf_restore_from_previous(int16_t* lsfq,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int16_t* past_quantizer_outputs[MA_NP + 1],
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int ma_predictor_prev)
> >> +{
> >> + ? ?int16_t* quantizer_output = past_quantizer_outputs[MA_NP];
> >> + ? ?int i,k;
> >> +
> >> + ? ?for (i = 0; i < 10; i++) {
> >> + ? ? ? ?int tmp = lsfq[i] << 15;
> >> +
> >> + ? ? ? ?for (k = 0; k < MA_NP; k++)
> >> + ? ? ? ? ? ?tmp -= past_quantizer_outputs[k][i] * cb_ma_predictor[ma_predictor_prev][k][i];
> >> +
> >> + ? ? ? ?quantizer_output[i] = ((tmp >> 15) * cb_ma_predictor_sum_inv[ma_predictor_prev][i]) >> 12;
> >> + ? ?}
> >> +}
> >
> > this code still is practically the same as lsf_decode()
> 
> It does opposite calculation.
> Then lsf_decode calculates LSF from MA predictor and past_quantizer_output.
> lsdf_restore_from_previous calculated past_quantizer output from last
> LSF and _last
> good_ MA predictor. Merging them together will not make understanding
> things better, imho.

if the code restores a value that was previously known, simply storing that
value in the context seems a cleaner choice
if not the word "restore" does not belong into this anywhere


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20090625/2da63681/attachment.pgp>