[FFmpeg-devel] [PATCH] Frame erasure case of pitch delay decoding

[Vladimir Voroshilov]

2009/6/28 Vladimir Voroshilov <voroshil at gmail.com>:
> 2009/6/28 Michael Niedermayer <michaelni at gmx.at>:
>> On Sat, Jun 27, 2009 at 09:54:06AM +0700, Vladimir Voroshilov wrote:
>>> 2009/6/27 Michael Niedermayer <michaelni at gmx.at>:
>>> > On Sat, Jun 27, 2009 at 01:49:49AM +0700, Vladimir Voroshilov wrote:
>>> >> Updated patch.
>>> >>
>>> >>
>>> >> --
>>> >> Regards,
>>> >> Vladimir Voroshilov ? ? mailto:voroshil at gmail.com
>>> >> JID: voroshil at gmail.com, voroshil at jabber.ru
>>> >> ICQ: 95587719
>>> >
>>> >> ?g729dec.c | ? ?7 ++++++-
>>> >> ?1 file changed, 6 insertions(+), 1 deletion(-)
>>> >> c66e04bc9abc50854d2bd35ac5299ae0e4b28637 ?0005-Frame-erasure-support-for-pitch-delay-decoding.176.patch
>>> >> From aa4b23873bc3c5e103a5756ade017558bda12961 Mon Sep 17 00:00:00 2001
>>> >> From: Vladimir Voroshilov <voroshil at gmail.com>
>>> >> Date: Thu, 11 Jun 2009 13:51:28 +0700
>>> >> Subject: [PATCH 05/25] Frame erasure support for pitch delay decoding
>>> >>
>>> >>
>>> >> diff --git ffmpeg-r19281/libavcodec/g729dec.c ffmpeg-r19281_v176/libavcodec/g729dec.c
>>> >> index b937235..f193123 100644
>>> >> --- ffmpeg-r19281/libavcodec/g729dec.c
>>> >> +++ ffmpeg-r19281_v176/libavcodec/g729dec.c
>>> >> @@ -306,7 +306,9 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *data_size,
>>> >> ? ? ? ? ?gc_1st_index ?= get_bits(&gb, format.gc_1st_index_bits);
>>> >> ? ? ? ? ?gc_2nd_index ?= get_bits(&gb, format.gc_2nd_index_bits);
>>> >>
>>> >> - ? ? ? ?if(!i) {
>>> >> + ? ? ? ?if (frame_erasure)
>>> >> + ? ? ? ? ? ?pitch_delay_3x ? = 3 * ctx->pitch_delay_int_prev;
>>> >> + ? ? ? ?else if(!i) {
>>> >> ? ? ? ? ? ? ?if (bad_pitch)
>>> >> ? ? ? ? ? ? ? ? ?pitch_delay_3x ? = 3 * ctx->pitch_delay_int_prev;
>>> >
>>> > if(!i || frame_erasure)
>>> >
>>> > also, i think bad_pitch and frame_erasure are redundant that is the same
>>> > either your frame is damaged or not
>>> > If you disagree, id like to hear which real life system uses g729 and
>>> > can suffer from single bit errors not passing through a FEC code
>>> > (that stuff might exist but my gut feeling says it does not, so unless
>>> > ?you have an example i prefer the simpler code)
>>>
>>> Afaik, bad_pitch is rsult of checking most significant part of input
>>> parameters: vector of quantizer
>>> along with switched MA predictor. Getting garbage here will cause
>>> large quality degradaion.
>>> On the other side remaining parts will bring far less noise.
>>> I can guess that having something data in frame (even with bad vector
>>> of quantizer) from the standpoint of G.279 authors is still better
>>> than entire frame erasure.
>>
>> you keep implicitly assuming that single bit errors happen, so let me
>> ask again, do you know of such a case? maybe its a common thing in some
>> scenario but if not a parity error in any part of the frame means that
>> the rest of the frame will likely be random as well
>
> Well spec says (4.1.6 Computation of the parity bit):
> "If this bit is not identical to transmitted parity bit, it is more
> likely that bit
> _errorS_ occured during transmission." And then spec describes how to
> compute pitch delay of the first subframe
> in this case (it is also clearly says that pitch ?delay for second
> subframe is calculated as in common case).
> This is the _only_ place when parity bit usage is described. Thus
> bad_pitch should affect only pitch_delay.
>
> On the other side spec does not say what to do with pitch delay if
> frame erasure is detected.
> Reference code uses the same computation (as for bad parity bit) for
> both first and second subframes.
> This is the only way to avoid usage of adaptive-codebook index in the
> second subframe.
>
>> did you run any tests of burst error, damaged sectors/packets in terms
>> of subjective quality for the different variants? has ITU performed
>> such tests? and if so are they documented somewhere?
>
> I did only tests with random frame_erasure flag. I didn't remember
> exact results.
> If i'm not wrong, most sample remains understandable with 50% frames erased
> and even more for some samples.
>
> I didn't ?test bad_pitch, except ?in "pitch" ITU test, which
> definitely tests only specific parts
> of code and nothing more.
>
> And i don't know any ITU test results you saying about.
>
>>> If you can see in case of frame erasure pitch delay is computed
>>> differently for both subframes, while bad_pitch
>>> keeps computing of pitch_delay for second subframe the same.
>>>
>>> And here we again comes to the question about bitexactness.
>>> If it is possible i prefer creating decoder which passes all ITU
>>> tests, and ONLY after that
>>> break it (decoder) by introducing desired optimizations.
>>
>> iam not stoping you from creating such decoder
>> but for ffmpeg i like to have an independant, optimal and conformant
>> implementation of the spec, if that includes bitexactness depends on
>> the spec requireing it and there being any advantage in breaking such
>> bitexactness to one of many implementations
>
> Hmm. About test vectors:
> ====
> This directory contains testvectors to validate the correct execution
>
> of the G.729 ANSI-C software (version 3.3). NOTE that these vectors
>
> are not part of a validation procedure. It is very difficult to design
>
> an exhaustive set of test vectors. Hence passing these vectors should
>
> be viewed as a minimum requirement, and is not a guarantee that the
>
> implementation is correct for every possible input signal.
>
>
> [snip]
>
> The testvectors were designed to provide as much coverage as possible
>
> in terms of parameters and algorithm. Below we indicate what parts of
>
> the algorithms are excercised. Note that none of these sequences
>
> provides an exhaustive coverage.
>
> ===
>
> "not part of a validation procedure", "should be viewed as minimum requirement"
> I'm in doubt again, whether passing test vector required or not for independent
> implementations.

OMG. Just find another thing. In Annex D (6.4k mode) D.7 paragraph also says:
The ANSI C code
represents the normative specification of this annex. The algorithmic
description given by the C
code shall take precedence over the texts contained in the main body
of G.729 and in Annex D.

Thus, when writing 6.4k decoder i should follow C code rather than specification
when they are saying different things, should i ?

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