[FFmpeg-devel] [PATCH] AAC Pulse data boundaries

Robert Swain robert.swain
Mon Sep 15 16:52:52 CEST 2008


2008/9/15 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Sep 15, 2008 at 12:24:10PM +0100, Robert Swain wrote:
>> 2008/9/15 Michael Niedermayer <michaelni at gmx.at>:
>> > On Mon, Sep 15, 2008 at 12:36:21AM +0100, Robert Swain wrote:
>> >> 2008/9/14 Alex Converse <alex.converse at gmail.com>:
>> >> > Currently the AAC pulse_data tool can try to apply pulses to offsets outside
>> >> > of the boundaries of the spectral coefficients. This leads to a crash
>> >> > (SIGSEGV) with a stream generated wit the following command:
>> >> > zzuf -s16 -r0.001:0.1 -b1193- <
>> >> > mpeg4audio-conformance/compressedMp4/al04_08.mp4 > fuzzed.mp4
>> >> >
>> >> > Attached is a patch to clip pulse positioning to 1023.
>> >>
>> >> The code changes look OK but is clipping the best solution to this or
>> >> should those pulses rather be discarded? Michael?
>> >>
>> >> It would seem to me that if there are multiple pulses, clipping them
>> >> could lead to multiple pulses being applied to position 1023. Is a
>> >> spike in the spectral data better than no spike or vice versa? My
>> >> guess is that ignoring incorrectly positioned pulses may be a better
>> >> option.
>> >
>> > Well as you ask :)
>> > Bitstream errors should cause the data until the next resync possibility
>> > to be discarded. It also should discard some of the pevious data as the
>> > bitstream error likely happened earlier.
>> > In practice that likely means replacing the current frame by silence, that
>> > is before the windowing. Or maybe to duplicate the last frame.
>>
>> If it's possible to do something better than discard everything and
>> output silence, I think we should. Is this the concept of error
>> concealment whose errors I have seen in h.264 and MPEG-4 streams as I
>> recall?
>
> no, see error_resilience.c
>
>
>>
>> > Just listen to a slightly damaged file to find out which solution sounds
>> > best. This really is the only way to be sure what is best ...
>>
>> For this particular issue (as there can only be 5 pulses maximum
>> anyway...), my gut tells me that continuing but discarding the invalid
>> pulse positions would be best.
>
> if you store 100 huffman vlc coded symbols and you know that symbol 99 can
> store 0..64 but only 0..6 are allowed.
> Then if you find a value of >6 in that last symbol you know this symbol
> is wrong but assuming the 98 symbols before are not is plain silly, as
> you didnt have any means to check their validity in the first place.
>
> Let us assume that, there is just a single 1 bit error in the whole frame.
> Let us further assume that the described 100 vlc symbols above are that
> frame.
>
> there now is a 1% chance for the error to be in the last symbol and a 99%
> chance for it to be before.
> if the error is in the last symbol the chance to detect it is high (in this
> example)
> if the error is before it, one of 2 things can happen
> A. The length of the damaged vlc matches the undamaged one, in which case
>   no desync will likely happen and no error will be detected, as nothing is
>   deteted we cant do anything here anyway and dont need to consider it.
> B. The length of the damaged vlc does not match the undamaged one, in which
>   case desync will likely happen and the last symbol will also very likely
>   not be within the valid 0..6 range. Here, as vlcs are read from incorrect
>   places in the bitstream the whole data after the error should be pretty
>   much trashed.
>
> Looking at it from the other side, a detected error in the last symbol is
> much more likely prior to it than in the last symbol itself. This is even
> more true if the damage is more than a bit as complete desync is more likely
> with larger errors.

I see. Thanks for the explanation. :)

>> I'll leave that to Alex to test, if he
>> wants. :)
>
> someone should test it ...

As we can't detect whether the rest of the bitstream within this frame
(or synched section) is valid, my proposal seems pointless now and not
worth testing. Unless we find an encoder that is creating streams with
invalid pulse positions. But then that encoder is broken and that's
not our problem. :)

Rob




More information about the ffmpeg-devel mailing list