On 2012-11-16 21:02:43 -0800, Ronald S. Bultje wrote: > Hi, > > On Fri, Nov 16, 2012 at 8:43 AM, Janne Grunau <[email protected]>wrote: > > > I'm not entirely convinced that is the the correct/best way to fix the > > problem with this sample. It crashes in draw_edges_10_c because the > > image format and parameters has just 8 bit per pixels. > > > > The main problem I see is that it does far too many context re-inits. > > > > Janne > > ---8<--- > > > > Fixes a crash in the fuzzed sample sample_varPAR.avi_s26638 with > > alternating bit depths. > > --- > > libavcodec/h264.c | 88 > > ++++++++++++++++++++++++++++++------------------------- > > 1 file changed, 48 insertions(+), 40 deletions(-) > > > > So ... I originally added code to not support changing bitdepth at all, > basically to prevent this and alike issues.
I doubt your check would have catched this. > E.g., what if the reference is 8bit but the next frame is 10bit? It is not really changing bit depth. It parses a SPS with modified bit depth and id due to fuzzing, it copies the just parsed SPS to h.sps and reinits function pointers according to this. Later h.sps is rewritten by the cached SPS with a different id taken from the PPS. The pixel format is derived from this SPS and our decoder state became inconsistent and explodes later when calling 10bpp draw_edges with the parameters derived from a 8bpp frame. We have to either check for this inconsistency or move the reinit closer to decoding when we have the SPS we're actually using for decoding. I decided to move the reinit of the function pointer since at least in this case it's safe and more error resilient. The broken SPS is never used. > So I wouldn't mind just erroring out altogether, unless you think this can > really be made to work? I mean, if we want to support reinits, with frame > threading, then we should just rewrite the init code and allow complete > reinits and be convinced it always works, like we are for ffvp8. A real change of bit depth, pixel format or frame size will always trigger and IDR-like reinit. This is the only sane way to support parameter changes and as Loren said covered by the spec. This patch still needs work. I'll send an update later. Janne _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
