dilagurung / libyuv

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

ARGBToUVRow_Unaligned_SSSE3 crash on Win32 #194

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Call ConvertToI420 with a MEDIASUBTYPE_RGB32 frame from camera.
2.
3.

What is the expected output? What do you see instead?

Call stack:

ARGBToUVRow_Unaligned_SSSE3(const unsigned char * src_argb0, int 
src_stride_argb, unsigned char * dst_u, unsigned char * dst_v, int width) Line 
1199    C++
ARGBToI420(const unsigned char * src_argb, int src_stride_argb, unsigned char * 
dst_y, int dst_stride_y, unsigned char * dst_u, int dst_stride_u, unsigned char 
* dst_v, int dst_stride_v, int width, int height) Line 838  C++
ConvertToI420(const unsigned char * sample, unsigned int __formal, unsigned 
char * y, int y_stride, unsigned char * u, int u_stride, unsigned char * v, int 
v_stride, int crop_x, int crop_y, int src_width, int src_height, int dst_width, 
int dst_height, libyuv::RotationMode rotation, unsigned int fourcc) Line 
1915    C++

The debugger lands on line 1199 of row_win.cc:
    /* step 1 - subsample 16x2 argb pixels to 8x1 */
    movdqu     xmm0, [eax]

Local variables:

eax 172115576   unsigned int

The parameters passed into ConvertToI420:

        dst_width   640 int
        inv_dst_height  -480    int
+       u   0x0a3c2ed8 ""   unsigned char *
        u_stride    320 int
+       v   0x0a3d5ad8 ""   unsigned char *
        v_stride    320 int
+       y   0x0a377ed8 ""   unsigned char *
        y_stride    640 int

What version of the product are you using? On what operating system?
Windows 8 Pro 64-bit on Microsoft Surface Pro

Please provide any additional information below.
This worked on previous versions of libyuv. I updated yesterday and notice the 
problem. All code is compiled as 32-bit.

Original issue reported on code.google.com by jul...@ottapp.com on 27 Feb 2013 at 12:27

GoogleCodeExporter commented 9 years ago
Parameters seems okay.  You didnt show sample, which is what is being read, but 
the pointer is in the vicinity of the yuv.
The SSSE3 didn't change, but an AVX2 version was added, and perhaps a bug was 
introduced at that point.

Investigating.

Original comment by fbarch...@chromium.org on 27 Feb 2013 at 1:00

GoogleCodeExporter commented 9 years ago
Current version is r583.
I synced back to r500 and compared ARGBToUVRow_Unaligned_SSSE3 and ARGBToI420 
and dont spot any differences.

To sync a specific version, syntax is like this:
svn checkout https://libyuv.googlecode.com/svn/trunk/ libyuv -r 500

Try setting an environment variable
set LIBYUV_DISABLE_ASM=1
and run your program using just C code.  That would help narrow if the problem 
is in the SSSE3 or something else.

You'd get better performance if the pointers were aligned to 16 bytes.

I attempted to repro with
>set LIBYUV_HEIGHT=480
>set LIBYUV_WIDTH=640
>out\release\libyuv_unittest --gtest_filter=*ARGBToI420*
Note: Google Test filter = *ARGBToI420*
[==========] Running 4 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 4 tests from libyuvTest
[ RUN      ] libyuvTest.ARGBToI420_Any
[       OK ] libyuvTest.ARGBToI420_Any (22 ms)
[ RUN      ] libyuvTest.ARGBToI420_Unaligned
[       OK ] libyuvTest.ARGBToI420_Unaligned (23 ms)
[ RUN      ] libyuvTest.ARGBToI420_Invert
[       OK ] libyuvTest.ARGBToI420_Invert (22 ms)
[ RUN      ] libyuvTest.ARGBToI420_Opt
[       OK ] libyuvTest.ARGBToI420_Opt (22 ms)
[----------] 4 tests from libyuvTest (96 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test case ran. (100 ms total)
[  PASSED  ] 4 tests.

It tests unaligned source, and inverted height (-480).  No problems found.

Original comment by fbarch...@chromium.org on 27 Feb 2013 at 2:22

GoogleCodeExporter commented 9 years ago
I disabled ASM.

The debugger now lands at line 314 of row_commom.cc at MAKEROWY(ARGB, 2, 1, 0, 
4).

        ARGBToUVRow 0x01485417 {Jittr.exe!_ARGBToUVRow_C}   void (const unsigned char *, int, unsigned char *, unsigned char *, int) *
+       dst_u   0x0b16b068 ""   unsigned char *
+       dst_v   0x0b17dc68 ""   unsigned char *
        height  480 int
+       src_argb    0x07134e18 <Error reading characters of string.>    const unsigned 
char *
        src_stride_argb -2560   int
        width   640 int
        y   0   int

One thing I noticed above is the src_stride_argb is negative. I'm not sure why 
it would be negative.

This code worked a couple of months ago then as mentioned I updated and it 
broke. My conversion functions match those from WebRTC.

I'm going to keep looking for something I may be missing...

Original comment by jul...@ottapp.com on 4 Mar 2013 at 1:38

GoogleCodeExporter commented 9 years ago
Thanks for your patience.  I haven't spotted anything unusual and I do test for 
this case.

DirectX captures ARGB upsidedown and indicates it with a negative height.
libyuv accepts negative height and implements it by starting on the last row of 
the source and using a negative stride.
libyuvTest.ARGBToI420_Invert tests this.

See line 790 in convert.cc
// Negative height means invert the image.              
if (height < 0) {                                       
  height = -height;                                     
  src_argb = src_argb + (height - 1) * src_stride_argb; 
  src_stride_argb = -src_stride_argb;                   
}                                                       

The crash in both SSSE3 and C code is on the fetch of an ARGB pixel.

In your case, 0x07134e18 should point to the first pixel of the last row.
for 640x480, width = 640 so stride = 2560.
size of the ARGB buffer should be 1228800 bytes
0x07134e18 - 640*(480-1)*4 = 0x7009818.  Is that what was passed in?

Re This code worked a couple of months ago.
Can you sync and rebuild against specific versions of libyuv to narrow down 
which version introduced the bug?

Original comment by fbarch...@chromium.org on 5 Mar 2013 at 1:29

GoogleCodeExporter commented 9 years ago
OK, I reverted to r231 and I have the same problem. I'm wondering if this could 
be because I am running 32-bit code on 64-bit windows? or Because I have 
upgraded to VS11? These are the only changes since the code previously worked 
and I don't have access to the old system. The old system was VS10 and 32-bit 
Windows.

Original comment by jul...@ottapp.com on 16 Mar 2013 at 5:43

GoogleCodeExporter commented 9 years ago
OK, sorry to post again so soon but I have additional information. I have a 
screen sharing functionality in the software that works off the same image 
format ARGB and goes through the identical code paths (rotation, conversion to 
i420 and then VP8 encode). This still works! :-) So it's related only to frames 
derived from the camera. Interesting...

Original comment by jul...@ottapp.com on 16 Mar 2013 at 5:59

GoogleCodeExporter commented 9 years ago
I've resolved the problem! Previously I was reallocating the video buffer in 
the device capture thread for each new frame. Apparently the Microsoft Surface 
didn't like this. Now I use a pre-allocated buffer and reuse it for every 
subsequent frame. I appreciate the help.

Original comment by jul...@ottapp.com on 16 Mar 2013 at 6:35

GoogleCodeExporter commented 9 years ago
Is there anything I could add to libyuv to catch this?
Was the allocation failing and the call passed a NULL?

Original comment by fbarch...@chromium.org on 18 Mar 2013 at 10:59

GoogleCodeExporter commented 9 years ago
The allocation was successful and the buffer was non-NULL. I'm not certain 
there is anything that could be done in libyuv.

Original comment by jul...@ottapp.com on 19 Mar 2013 at 6:41

GoogleCodeExporter commented 9 years ago
Closing as invalid - no bug found in libyuv.
Thanks for the update.

Original comment by fbarch...@chromium.org on 19 Mar 2013 at 7:25