[FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom

[Ronald S. Bultje]

Hi,

On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer <
michael at 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 <michaelni at gmx.at>
> > wrote:
> >
> > > From: Michael Niedermayer <michael at 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 <michael at 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).

Ronald