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

[Michael Niedermayer]

On Mon, Sep 15, 2008 at 07:48:30PM +0100, Robert Swain wrote:
> 2008/9/15 Michael Niedermayer <michaelni at gmx.at>:
> > On Sun, Sep 14, 2008 at 10:48:17PM -0400, Alex Converse wrote:
> >> On Sun, Sep 14, 2008 at 8:55 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> >>
> >> > 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.
> >> >
> >>
> >> I was under the impression that the MPEG-4 Visual decoder accepts all kinds
> >> of things that aren't quite legal.
> >
> > There are 2 different things
> > 1. invalid bitstreams due to buggy encoders
> > 2. invalid bitstreams due to bit/byte/block/sector/packet damage.
> >
> > Until we have seen a buggy encoder encoding invalid pulses its IMO
> > reasonable to assume that no such encoder exists, if one does we
> > will get a bugreport and can adapt our decoder.
> > And if needed use error_recognition to decide where to draw the line
> > between points 1 and 2.
> 
> How does this work? Count up invalid values and if the number of
> invalid values read exceeds some threshold then the frame gets
> discarded?

no,
If the file has been generated by a buggy encoder that is known to set
a specific field F to a invalid value, that should always be ignored
If the file has not been generated by such encoder a invalid value in field
F is an error that should always trigger the error concealment code, which may
replace the frame or part of it by some estimate. For video that estimate may
be formed by the surrounding blocks from undamaged slices or the previous
frame. The code in libavcodec/error_resilience.c analyzes a frame to determine
which estimate is better in the current situation.
For h263/mpeg4/mpeg1/mpeg2 at least i can say with certainity that a single
error practically always completely destroys everything following until the
next slice. so theres no sense in counting, >0 means the following stuff
is useless.

How do we know if the encoder was buggy, well first one assumes that the
encoder is not buggy.
And thats enough until there IS a buggy encoder.

If the unlikly case happens and one exists, bugreports will then point to
files that where encoded with it. 
For video at least its common practice to store the encoder name and version
in there, be that as fourcc, or user data. So alot can be detected and
dealt with optimally.

also error_recognition exists as a generic threshold to choose between
error concealment and ignoring. That is somewhat orthognal to above
The user can use it to tell the decoderf what to do with errors

that is IIRC approximately:
0                -> nothing is an error
FF_ER_CAREFUL    -> only things which have never been seen in undamaged files
                    are errors
FF_ER_COMPLIANT  -> things that the spec does not allow are errors
FF_ER_AGGRESSIVE -> things that the spec does allow but that make no sense
                    are errors (like negative brightness)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- 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/20080915/6cbbdf7a/attachment.pgp>