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

Thu Jun 25 13:20:53 CEST 2009

2009/6/25 Michael Niedermayer <michaelni at gmx.at>:
> On Thu, Jun 25, 2009 at 09:22:33AM +0700, Vladimir Voroshilov wrote:
>> 2009/6/25 Michael Niedermayer <michaelni at gmx.at>:
>> > On Wed, Jun 24, 2009 at 08:18:23PM +0700, Vladimir Voroshilov wrote:
>> >> 2009/6/24 Michael Niedermayer <michaelni at gmx.at>:
>> >> > On Sat, Jun 20, 2009 at 02:26:59PM +0700, Vladimir Voroshilov wrote:
>> >> >
>> >> > [...]
>> >> >> diff --git ffmpeg-r19218/libavcodec/g729data.h ffmpeg-r19218_v163/libavcodec/g729data.h
>> >> >> index a70c705..e202331 100644
>> >> >> --- ffmpeg-r19218/libavcodec/g729data.h
>> >> >> +++ ffmpeg-r19218_v163/libavcodec/g729data.h
>> >> >> @@ -275,4 +275,14 @@ static const int16_t cb_ma_predictor_sum[2][10] = { /* (0.15) */
>> >> >> ? ?{14585, 18333, 19772, 17344, 16426, 16459, 15155, 15220, 16043, 15708}
>> >> >> ?};
>> >> >>
>> >> >> +/**
>> >> >> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 2^27
>> >> >> + * ff_g729_cb_ma_predictor_sum_inv[j][i] = ---------------------------------
>> >> >> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ff_g720_cb_ma_predictor_sum[j][i]
>> >> >> + */
>> >> >
>> >> > my guess would be that this is also not really "="
>> >> >
>> >> >
>> >> > [...]
>> >> >> - ? ?ma_predictor ? ? = get_bits(&gb, 1);
>> >> >> - ? ?quantizer_1st ? ?= get_bits(&gb, VQ_1ST_BITS);
>> >> >> - ? ?quantizer_2nd_lo = get_bits(&gb, VQ_2ND_BITS);
>> >> >> - ? ?quantizer_2nd_hi = get_bits(&gb, VQ_2ND_BITS);
>> >> >> +
>> >> >> + ? ?if(frame_erasure) {
>> >> >> + ? ? ? ?skip_bits(&gb, 1 + VQ_1ST_BITS + VQ_2ND_BITS + VQ_2ND_BITS);
>> >> >> +
>> >> >> + ? ? ? ?lsf_restore_from_previous(ctx->lsfq, ctx->past_quantizer_outputs,
>> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ctx->ma_predictor);
>> >> >> + ? ?} else {
>> >> >> + ? ? ? ?uint8_t quantizer_1st; ? ?///< first stage vector of quantizer
>> >> >> + ? ? ? ?uint8_t quantizer_2nd_lo; ///< second stage lower vector of quantizer (size in bits)
>> >> >> + ? ? ? ?uint8_t quantizer_2nd_hi; ///< second stage higher vector of quantizer (size in bits)
>> >> >> +
>> >> >> + ? ? ? ?ctx->ma_predictor = get_bits(&gb, 1);
>> >> >> + ? ? ? ?quantizer_1st ? ? = get_bits(&gb, VQ_1ST_BITS);
>> >> >> + ? ? ? ?quantizer_2nd_lo ?= get_bits(&gb, VQ_2ND_BITS);
>> >> >> + ? ? ? ?quantizer_2nd_hi ?= get_bits(&gb, VQ_2ND_BITS);
>> >> >>
>> >> >
>> >> > reindent besides i dont like the change, its useless to replace them
>> >> > by skip
>> >>
>> >> I though this version is more clean due to localized usage of variables.
>> >> It also shows that in erasure case reading of quantizer values is not used.
>> >>
>> >> here is my early version without skip, but with two variables per MA
>> >> predictor (current/prev values).
>> >
>> > [...]
>> >
>> >> @@ -162,6 +164,27 @@ static void lsf_decode(int16_t* lsfq, int16_t* past_quantizer_outputs[MA_NP + 1]
>> >> ? ? ?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;
>> >> + ? ?}
>> >> +
>> >> + ? ?/* Rotate past_quantizer_outputs. */
>> >> + ? ?memmove(past_quantizer_outputs + 1, past_quantizer_outputs, MA_NP * sizeof(int16_t*));
>> >> + ? ?past_quantizer_outputs[0] = quantizer_output;
>> >> +}
>> >
>> > this code is duplicated
>>
>> Fixed.
>>
>>
>> --
>> Regards,
>> Vladimir Voroshilov ? ? mailto:voroshil at gmail.com
>> JID: voroshil at gmail.com, voroshil at jabber.ru
>> ICQ: 95587719
>
>> ?g729data.h | ? ?5 +++++
>> ?g729dec.c ?| ? 35 +++++++++++++++++++++++++++++++----
>> ?2 files changed, 36 insertions(+), 4 deletions(-)
>> 9b703bec58c166bc06b1ffbb94f4297cb7499639 ?0001-Frame-erasure-support-for-LSF-decoding.171.patch
>> From 70858592b30d234b8d64be6950d8038e6d36bbc7 Mon Sep 17 00:00:00 2001
>> From: Vladimir Voroshilov <voroshil at gmail.com>
>> Date: Thu, 11 Jun 2009 12:30:46 +0700
>> Subject: [PATCH 01/25] Frame erasure support for LSF decoding
>>
>>
>> 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.

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

Note that LSF values are the same as in previous frame.

> also what is lsf_restore_from_previous supposed to mean?
> the function _GUESSES_ values of a damaged (erased) frame nothing is restored
> and a russian-english dictionary might be a good idea if you dont see
> why restored is just the wrong word
> of course i might also be misunderstanding the spec and maybe this is used
> for more than damaged frames but thats how i understand it currently

You are understanding right, this routine is used only when frame is
detected as damaged.
But since LSF computation code requires past quantizer outputs from
past four frames,
we need to compute (yes, perhaps, this can be called "guess") value
related for current damaged frame
too.

Specs says only "Since the codeword is not available for the current frame m,
it is computed from the repeated LSF parameters wi and the predictor memory"

Perhaps something like "compute_quantizer_from_lsf" ?

>
> [...]
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFKQ0bSYR7HhwQLD6sRAhqsAJwJ0jD+0KRsZt+R0t9Wh/njhqbnJACfbgCv
> 1Rl4E4qvO1vhWL3cQjv80PY=
> =2grI
> -----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