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

[Michael Niedermayer]

On Thu, Jan 14, 2016 at 05:34:25PM +0100, Michael Niedermayer 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)
> 
> the yuv tables likely are still not large enough, but a theoretical
> limit exists due to intermediate being 16bit -> hscale -> 32bit >>19
> i tried av_clip_uint8 but it was alot slower

speedloss for clip was 1% overall that is time ffmpeg ...
(thats with cpuflags 0 as the code isnt used otherwise)

the clip check was with if((y1|y2|u|v) & CONST)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160114/1085b1ae/attachment.sig>