skufog / libyuv

Automatically exported from code.google.com/p/libyuv
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Read after last pixel in ARGScale() using bilinear filter and odd dest size #261

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Allocate source buffer of *exactly* 320*240*4 bytes (no padding at the end 
required) & destination buffer of 353*264*4 bytes
2. Call ARGBScale(src, 320*4, 320, 240, dst, 353*4, 353, 264, kFilterBilinear)
3. Access violation in ScaleARGBFilterCols()

What is the expected output? What do you see instead?
Expected: correct resize
Actual: access violation

What version of the product are you using? On what operating system?
r745 
Visual Studio 32 bits on Windows 2003 & 7

Please provide any additional information below.

Patch attached to fix ScaleARGBFilterCols_C() and ScaleARGBFilterCols_SSSE3() 
32 bits version
The patch does Not include ScaleARGBFilterCols_SSSE3() 64 bits version

No issue with kFilterNone
Not tested with kFilterBox

Valgrind/ASAN tests should be added for this case too

Original issue reported on code.google.com by alt...@gmail.com on 26 Aug 2013 at 3:31

Attachments:

GoogleCodeExporter commented 9 years ago
This bug also cause the value of each last destination row pixel to be wrong: 
it is computed using last source row pixel and first source pixel of *next* row 
(assuming stride = width)

Original comment by alt...@gmail.com on 30 Aug 2013 at 7:03

GoogleCodeExporter commented 9 years ago
The expected result is normally that the last pixel or row is used 100% and 
last pixel + 1 is used 0%.
With clipping, this is not always the case - the last clipped pixel destination 
pixel could be any source pixels.  The patch as is, does not account for 
clipping.
Can you add a pixel of padding?

Original comment by fbarch...@google.com on 11 Sep 2013 at 2:51

GoogleCodeExporter commented 9 years ago
Sorry for my late answer.

I don't see the issue with clipping. Can you provide an example please?

> Can you add a pixel of padding?
Not in the use case I encountered: the input buffer is allocated by an external 
library with a fixed size.

Original comment by alt...@gmail.com on 30 Sep 2013 at 2:24

GoogleCodeExporter commented 9 years ago
util/convert tool as well argb tile scale unittest, demonstrate a tiled scale.
each small block of destination comes from a region of subpixel accurate 
source, using clipping.

The change as is, doesnt solve the problem for all scale factors, and 
introduces an issue with tiling.
More thought required.

Original comment by fbarch...@google.com on 13 Oct 2013 at 3:25

GoogleCodeExporter commented 9 years ago
In r895 I've plugged in CIF to/from QVGA and somewhat tracked down the problem.
Short term fix is for bilinear to essentially compute slope using (width - 
1.0001) so the last pixel will be filtering width - 2 and width - 1 with nearly 
100% on width - 1.  Should have a fix today.

In the scaling down case, it would be easy to extrude the edge, but its less 
easy on the upsample.  So that will come later.  A complication of the extrude 
is it only should be done when clipping against the image edge.  This will take 
about a week, and I may want to refactor the scalers to share more code before 
doing it.

Original comment by fbarch...@google.com on 5 Dec 2013 at 10:37

GoogleCodeExporter commented 9 years ago
r898 fixes for ARGB.
YUV scaler needs the same change, so I'll keep the bug open.

Original comment by fbarch...@google.com on 6 Dec 2013 at 10:58

GoogleCodeExporter commented 9 years ago
r905 resolves a vertical step beyond max_y.
All known overread issues resolved.

Original comment by fbarch...@google.com on 9 Dec 2013 at 9:59