Hi, On Thu, Jan 14, 2016 at 12:06 PM, Michael Niedermayer < mich...@niedermayer.cc> wrote:
> On Thu, Jan 14, 2016 at 11:42:47AM -0500, Ronald S. Bultje wrote: > > Hi, > > > > On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer < > > mich...@niedermayer.cc> wrote: > > > > > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > > > Hi, > > > > > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer < > michae...@gmx.at> > > > > wrote: > > > > > > > > > From: Michael Niedermayer <mich...@niedermayer.cc> > > > > > > > > > > This makes SWS more robust > > > > > Fixes: > > > > > > > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > > > Fixes: out of array read > > > > > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > > > --- > > > > > libswscale/swscale_internal.h | 2 +- > > > > > libswscale/yuv2rgb.c | 88 > > > > > ++++++++++++++++++++--------------------- > > > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB > in > > > > swscale, probably through automated testing of ffmpeg.exe -i > file.264 -f > > > > null -. Can you elaborate on what happens exactly? I guess what I'm > > > trying > > > > to get at is, what's the input format (I'm going to assume it's > yuv420), > > > > what's the output (maybe rgb24?), why is it converting like that (is > the > > > > 264 changing pixfmt from frame to frame?), are the coefficients > defaults > > > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what > _are_ > > > > the coefficients, what coefficients can swscale use in these > functions, > > > and > > > > what are the theoretical bounds of the index in each array based on > these > > > > coefficients? > > > > > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 > in > > > > each of these tables? How do we verify that it is correct? I remember > > > Jason > > > > tried to speed up av_clip_uint8() at some point by making it a > table, and > > > > we had to revert the use of that in many places (e.g. idcts) because > we > > > > cannot give a theoretical limit on input values, i.e. the table would > > > have > > > > to be infinitely long. I'm trying to figure out if that's the case > here > > > > also. > > > > > > input was IIRC 10bit YUV (probably with out of range values) > > > > > > Ah! This gets very interesting. OK, so then it all makes a lot of sense. > So > > ... Is this valid input? I mean, the h264 decoder clips the output in > each > > stage (prediction, idct, loopfilter) to fit within the range, no? Or is > > this something like PCM or whatever where we don't clip? Should we? > > > > I'm not saying we shouldn't allow clipping of input values in swscale > also, > > but perhaps we could have a flag that says that the input is already > > clipped (similar to swresample). > > possibly, but that might be hard to get right > one path for too big values is uinitialized memory, our error > concealment code doesnt support some corner cases so it doesnt always > clean all unset set stuff up, memory is also IIRC not cleared after > alloc > > my reasoning for fixing it in swscale was that users will expect > sws not to crash from out of range input samples Uninitialized data in output frames seems like a pretty big deal to me regardless of bugs in swscale, no? Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
