[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [4/7] - G.729 core

[Vladimir Voroshilov]

2008/10/17 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Oct 17, 2008 at 03:14:27PM +0700, Vladimir Voroshilov wrote:
>> 2008/10/17 Michael Niedermayer <michaelni at gmx.at>:
>>
>> >> diff --git ffmpeg-r15432.orig/libavcodec/g729dec.c ffmpeg-r15432.mod/libavcodec/g729dec.c
>> >> index 6bd814f..74b0665 100644
>> >> --- ffmpeg-r15432.orig/libavcodec/g729dec.c
>> >> +++ ffmpeg-r15432.mod/libavcodec/g729dec.c
>> >
>> >> @@ -30,6 +30,9 @@
>> >>  #include "libavutil/avutil.h"
>> >>  #include "bitstream.h"
>> >>
>> >> +#define FRAC_BITS 15
>> >> +#include "mathops.h"
>> >> +
>> >>  #include "g729.h"
>> >>  #include "lsp.h"
>> >>  #include "acelp_math.h"
>> >
>> > this is unflexible and confusing, the used shift should not be file global
>> >
>> > [...]
>> >> +static const G729_format_description formats[] =
>> >> +{
>> >> +  {8000, 10, 160, 13, 1018156},
>> >
>> >> +// Note: may not work
>> >> +  {4400, 11, 176, 17, 1021547},
>> >> +};
>> >
>> > elaborate on "may" please
>> > if it does not work, it probably should be removed from the patch
>> > or fixed so it does work.
>> > if it does work what does the comment mean?
>> > and if it has not been tested, test it.
>>
>> 4400 mode is (i assume) Chinese extension of G.729. it is not covered
>> by official specification.
>> And since i have no samples i can't test it. This is why i put those comment.
>
> What makes you think this format even exists?
> Is there any source code for a decoder?
> Or a specification?
> Or is it mentioned in any manual for any hardware?
> Iam just a little curious where this stuff comes from ...

As Benjamin already said it was reversed from windows binary.

>> According to above the best way will be just dropping it completely
>> (but i whould prefer keeping format structure as is, because it will
>> be necessary when G.729 will be added).
>>
>
>> I am also not against dropping BFI code, since i feel myself having
>> poor knowledge to compare it with other frame erasure algorithms.
>> (But with BFI speech is understandable with 50% of input packets loss,
>> afair. I can made this simple test again, if you wish).
>
> and is it better or worse than repeating the previous packet?

I've made dumb test: each second frame was assumed "bad".

Original sound (all samples are raw 8kHz, 1 channel audio):
http://doors.univer.omsk.su/~voroshil/downloads/SPEECH.BIT.ffmpeg.sw

Repeating previous "good" frame:
http://doors.univer.omsk.su/~voroshil/downloads/SPEECH.BIT.ffmpeg.sw.fc

Frame erasure flag for "bad" frame:
http://doors.univer.omsk.su/~voroshil/downloads/SPEECH.BIT.ffmpeg.sw.fe

The latter sounds a bit better for me even if it
quieter (since algorithm reduces signal power on broken frame).

>> >
>> > [...]
>> >> @@ -138,6 +531,108 @@ static inline int g729_get_parity(uint8_t value)
>> >>                  14,
>> >>                  ctx->subframe_size);
>> >>
>> >> +        memcpy(synth, ctx->syn_filter_data, 10 * sizeof(int16_t));
>> >
>> > there are 2 memcpies copying this around, i dont see why it would need 2.
>>
>> I'll try to describe this as much as i understand it.
>>
>> Well... Synthesis filter can be written as
>> in[i]=sum{a[k]*out[i-k]},k=0..filter_order, i=0..subframe_size-1
>> where a[k] - filter coefficients, a[0] == 1, in[i] - input, out[i] -
>> output signal data
>>
>> Due to optimization, a[0] is eliminated and filter (in code) is implemented as:
>> buf[i]=buf[i] - sum{a[k]*buf[i-1-k]}, k=0..filter_order-1
>>
>> Thus, i need a buffer of length subframe_size+(filter_order-1).
>> But... the top of this buffer must contains k-1 samples from the end
>> of previous filter run result.
>> Brrr. I mean:
>> buffer after Nth call:
>> (0),..,(k-1),(k),..,[(subframe_size-k),..,(subframe_size-1)]
>>
>> buffer before (N+1)th  call:
>> [(0),..,(k-1)], (the rest is garbage yet)
>>
>> data in brackets must be the same.
>
> fine, so much i knew alraedy, that makes it one memcpy from the end to the
> start, not 2

This is true only when one filter is used (see below).

>> When synthesis filter is single filter working with this buffer, there
>> is enough one memmove.
>> Unfortunately there are also high-pass filter and postfilter, which
>> also changes buffer.
>> So, i have to save synthesis filter output data before passing them
>> to below filters and restore saved
>> data back before next call to synthesis filter.
>
> IIRC (ive not rechecked the code)
> there is data for 2 subframes
> [0..k] frame0 frame1
> by the time frame1 is done the memcpy would move the data from frame1
> over frame0, i thought this would not affect subsequent steps run over
> frame1?

Let's assume this order (subframe1 and subframe2 are processed by two
iterations of "for" loop inside one decoder call):
frame0_subframe1,frame0_subframe2,frame1_subframe1,frame1_subframe2

Assume also that we are just finished synthesis filtering of
frame1_subfram2 and want to save data.

If you put memmove before high-pass filter  you'll pass wrong input to
it (you'll pass just memmove'd end of the frame1_subframe2 instead of
expected end of frame0_subframe2).
If you put memmove after high-pass filter, synthesis filter will
receive (in next iteration) the end of frame1_subframe2 (as it
expects) but data will be already modified by high-pass filter.

In any case you'll broke either synthesis or high-pass filter.


-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719