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

[Alex Converse]

On Mon, Sep 15, 2008 at 9:10 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> 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.
>

What should be done about a corrupt/invalid PCE? It sets the
parameters for decoding the rest of the file.
Right now we bail from the PCE but then resync and use a half corrupt
half initialized PCE for the rest of the decoding which is incredibly
wrong and leads to segfaults in the later decoding stages, and yes I
have a stream that does this.
[...]

--Alex