[FFmpeg-devel] [PATCH] G.729 Frame erasure support for fixed-codebook vector decoding
Michael Niedermayer
michaelni
Sun Jun 28 22:36:06 CEST 2009
On Sun, Jun 28, 2009 at 11:34:51AM +0700, Vladimir Voroshilov wrote:
> 2009/6/28 Michael Niedermayer <michaelni at gmx.at>:
> > On Sun, Jun 28, 2009 at 02:01:54AM +0700, Vladimir Voroshilov wrote:
[...]
> > Also to say it in case i didnt yet, iam not arguing for any specific thing
> > or even for ignooring bitexactness i just think the arguments you use to
> > make design decissions are the wrong ones. I mean like
> > "lets keep it bitexact because its easier" vs. "ive tested your suggestions
> > and they sound worse"
>
> Nope. "it easy to track introduced bugs at this stage".
> Also I don't understand what should i treat as "worse".
> Breaking bitexactnes is worse for me but not for you.
>
> I'm testing each of your suggestion and trying to argue why i don't like it,
> instead of simple "it is worse" (at least i'm sure i do). I know that you
> never asks to change code without a reason.
>
> And as i already said, hearing the result is unreliable test for me.
> I'm not musician with perfect hearing. I'm not able to find exact
> place in code containing
> a bug after spotting one extra "click" in decoded sound.
if you hear a click you know there is a bug so you could go back to the
previous revission where you did not hear the click. Its a simple thing,
test each with ears and a PSNR tool and problems should be spoted quickly
>
> P.S. looks like discussing patch converts into long flaming :(
yes
> I'm sorry for this.
>
> Returing to the first you comment:
>
>
> >> @@ -224,6 +225,9 @@ static av_cold int decoder_init(AVCodecContext * avctx)
> >> ctx->lsp[1] = ctx->lsp_buf[1];
> >> memcpy(ctx->lsp[0], lsp_init, 10 * sizeof(int16_t));
> >>
> >> + /* random seed initialization */
> >> + ctx->rand_value = 21845;
>
> >the word pointless comes to mind, at least if this is just used for
> >frame erasure handling
>
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -336,6 +340,15 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> >> /* Round pitch delay to nearest (used everywhere except ff_acelp_interpolate). */
> >> pitch_delay_int = (pitch_delay_3x + 1) / 3;
> >>
> >> + if (frame_erasure) {
> >> + ctx->rand_value = g729_prng(ctx->rand_value);
> >> + fc_indexes = ctx->rand_value & ((1 << format.fc_indexes_bits) - 1);
> >> +
> >> + ctx->rand_value = g729_prng(ctx->rand_value);
> >> + pulses_signs = ctx->rand_value & ((1 << format.fc_signs_bits) - 1);
> >> + }
>
> > a single call to the prng is probably enough also is the & really needed?
>
> As i said before, seed initialization as long as double call to
> g729_prng and "&" are defined
> in main body of G.729, part 4.4.4 "Generation of the replacement
> excitation". Is this
> enough reason to keep them ?
no, its illogic for the handling of damaged frames to require specific bits
of a specific PRNG to be used
that said, are the & needed or not?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20090628/9c438105/attachment.pgp>
More information about the ffmpeg-devel
mailing list