diff options
author | wolenetz@chromium.org <wolenetz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-14 07:32:58 +0000 |
---|---|---|
committer | wolenetz@chromium.org <wolenetz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-14 07:32:58 +0000 |
commit | d4be63804ec2563d317a8c8fe1313ba6fa02608c (patch) | |
tree | f568dab859eb5ddd7aee8c14b5b1c4a5aa60fdb3 /media | |
parent | c9c89a156b8769fab8b25f64343ae2bc89706750 (diff) | |
download | chromium_src-d4be63804ec2563d317a8c8fe1313ba6fa02608c.zip chromium_src-d4be63804ec2563d317a8c8fe1313ba6fa02608c.tar.gz chromium_src-d4be63804ec2563d317a8c8fe1313ba6fa02608c.tar.bz2 |
Change prototypes to yuv_convert_simd_x86 yasm and equivalent C to portably sign extend int args
Inspection of stack prepared by caller shows it is indeed copying dword, not qword, for 32-bit arguments on Win64, and the background stack is not zeroed.
Incorrect function result, access violations or worse (not-immediately caught side effect of corrupted memory within process address space) could easily result from current win64 usage of these routines.
One solution, implemented here, is to change the function prototypes for the yasm routines to take ptrdiff_t arguments instead of int args. The C version of the equivalent code needs matching prototype (due to the choose proc routines), so they are updated too.
Another, probably slightly less performant, solution would be to change the asm routines to conditionally sign-extend when loading int (32-bit) parameters on x64. None of these routines currently use int passed in directly as register; I don't know if zero-extension of registers would be required if they did.
Yet another solution would be to switch to VS2012 and redo the yasm routines using compiler intrinsics completely (not supported in VS2010).
Note, some other media_unittests, namely those involving ffmpeg, currently give access violation on Win64. There is a separate ffmpeg fix in progress https://gerrit.chromium.org/gerrit/#/c/43063/. With PS1 or PS2 applied along with the ffmpeg fix, Win64 media_unittests pass.
Detail:
The following are sign-extended in prototype:
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertARGBToYUVRow_SSSE3
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertRGBToYUVRow_SSSE3
simd/convert_yuv_to_rgb_mmx.asm:%define SYMBOL ConvertYUVToRGB32Row_MMX
simd/convert_yuv_to_rgb_sse.asm:%define SYMBOL ConvertYUVToRGB32Row_SSE
simd/scale_yuv_to_rgb_mmx.asm:%define SYMBOL ScaleYUVToRGB32Row_MMX
simd/scale_yuv_to_rgb_sse.asm:%define SYMBOL ScaleYUVToRGB32Row_SSE
simd/scale_yuv_to_rgb_sse2_x64.asm:%define SYMBOL ScaleYUVToRGB32Row_SSE2_X64
simd/linear_scale_yuv_to_rgb_mmx.asm:%define SYMBOL LinearScaleYUVToRGB32Row_MMX
simd/linear_scale_yuv_to_rgb_sse.asm:%define SYMBOL LinearScaleYUVToRGB32Row_SSE
simd/linear_scale_yuv_to_rgb_mmx_x64.asm:%define SYMBOL LinearScaleYUVToRGB32Row_MMX_X64
The following C versions and the corresponding typedefs also needed updating to match:
ConvertYUVToRGB32Row_C
ScaleYUVToRGB32Row_C
LinearScaleYUVToRGB32Row_C
The following yasm routines have no C prototype, nor do they appear to be used anywhere. I updated their comments though to match the same prototype change that would be necessary.
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertARGBToYUVEven_SSSE3
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertARGBToYUVOdd_SSSE3
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertRGBToYUVEven_SSSE3
simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL ConvertRGBToYUVOdd_SSSE3
BUG=174160
TEST=with https://gerrit.chromium.org/gerrit/#/c/43063/ also applied, all win64 media_unittests non-disabled test cases pass
R=hclam@chromium.org
R=dalecurtis@chromium.org
R=rbultje@chromium.org
CC=jschuh@chromium.org
CC=scherkus@chromium.org
Review URL: https://chromiumcodereview.appspot.com/12211067
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182407 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
-rw-r--r-- | media/base/simd/convert_rgb_to_yuv_ssse3.asm | 12 | ||||
-rw-r--r-- | media/base/simd/convert_rgb_to_yuv_ssse3.h | 10 | ||||
-rw-r--r-- | media/base/simd/convert_yuv_to_rgb.h | 52 | ||||
-rw-r--r-- | media/base/simd/convert_yuv_to_rgb_c.cc | 10 | ||||
-rw-r--r-- | media/base/simd/convert_yuv_to_rgb_mmx.asm | 2 | ||||
-rw-r--r-- | media/base/simd/convert_yuv_to_rgb_sse.asm | 2 | ||||
-rw-r--r-- | media/base/simd/linear_scale_yuv_to_rgb_mmx.asm | 4 | ||||
-rw-r--r-- | media/base/simd/linear_scale_yuv_to_rgb_mmx_x64.asm | 6 | ||||
-rw-r--r-- | media/base/simd/linear_scale_yuv_to_rgb_sse.asm | 4 | ||||
-rw-r--r-- | media/base/simd/scale_yuv_to_rgb_mmx.asm | 4 | ||||
-rw-r--r-- | media/base/simd/scale_yuv_to_rgb_sse.asm | 4 | ||||
-rw-r--r-- | media/base/simd/scale_yuv_to_rgb_sse2_x64.asm | 4 |
12 files changed, 67 insertions, 47 deletions
diff --git a/media/base/simd/convert_rgb_to_yuv_ssse3.asm b/media/base/simd/convert_rgb_to_yuv_ssse3.asm index 201f558..b59362b 100644 --- a/media/base/simd/convert_rgb_to_yuv_ssse3.asm +++ b/media/base/simd/convert_rgb_to_yuv_ssse3.asm @@ -243,7 +243,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertARGBToYUVRow_SSSE3 %define PIXELSIZE 4 @@ -256,7 +256,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertRGBToYUVRow_SSSE3 %define PIXELSIZE 3 @@ -269,7 +269,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertARGBToYUVEven_SSSE3 %define PIXELSIZE 4 @@ -282,7 +282,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertARGBToYUVOdd_SSSE3 %define PIXELSIZE 4 @@ -295,7 +295,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertRGBToYUVEven_SSSE3 %define PIXELSIZE 3 @@ -308,7 +308,7 @@ ; uint8* y, ; uint8* u, ; uint8* v, -; int width); +; ptrdiff_t width); ; %define SYMBOL ConvertRGBToYUVOdd_SSSE3 %define PIXELSIZE 3 diff --git a/media/base/simd/convert_rgb_to_yuv_ssse3.h b/media/base/simd/convert_rgb_to_yuv_ssse3.h index 84557d1..92144c9 100644 --- a/media/base/simd/convert_rgb_to_yuv_ssse3.h +++ b/media/base/simd/convert_rgb_to_yuv_ssse3.h @@ -13,19 +13,25 @@ extern "C" { // instructions so we can call them from C++ code. These functions are // implemented in "convert_rgb_to_yuv_ssse3.asm". +// We use ptrdiff_t instead of int for yasm routine parameters to portably +// sign-extend int. On Win64, MSVC does not sign-extend the value in the stack +// home of int function parameters, and yasm routines are unaware of this lack +// of extension and fault. ptrdiff_t is portably sign-extended and fixes this +// issue on at least Win64. + // Convert a row of 24-bit RGB pixels to YV12 pixels. void ConvertRGBToYUVRow_SSSE3(const uint8* rgb, uint8* y, uint8* u, uint8* v, - int width); + ptrdiff_t width); // Convert a row of 32-bit RGB pixels to YV12 pixels. void ConvertARGBToYUVRow_SSSE3(const uint8* argb, uint8* y, uint8* u, uint8* v, - int width); + ptrdiff_t width); #ifdef __cplusplus } diff --git a/media/base/simd/convert_yuv_to_rgb.h b/media/base/simd/convert_yuv_to_rgb.h index 164ad11..d05f039 100644 --- a/media/base/simd/convert_yuv_to_rgb.h +++ b/media/base/simd/convert_yuv_to_rgb.h @@ -59,70 +59,78 @@ void ConvertYUVToRGB32_MMX(const uint8* yplane, // Assembly functions are declared without namespace. extern "C" { +// We use ptrdiff_t instead of int for yasm routine parameters to portably +// sign-extend int. On Win64, MSVC does not sign-extend the value in the stack +// home of int function parameters, and yasm routines are unaware of this lack +// of extension and fault. ptrdiff_t is portably sign-extended and fixes this +// issue on at least Win64. The C-equivalent RowProc versions' prototypes +// include the same change to ptrdiff_t to reuse the typedefs. + typedef void (*ConvertYUVToRGB32RowProc)(const uint8*, const uint8*, const uint8*, uint8*, - int); + ptrdiff_t); + typedef void (*ScaleYUVToRGB32RowProc)(const uint8*, const uint8*, const uint8*, uint8*, - int, - int); + ptrdiff_t, + ptrdiff_t); void ConvertYUVToRGB32Row_C(const uint8* yplane, const uint8* uplane, const uint8* vplane, uint8* rgbframe, - int width); + ptrdiff_t width); void ConvertYUVToRGB32Row_MMX(const uint8* yplane, const uint8* uplane, const uint8* vplane, uint8* rgbframe, - int width); + ptrdiff_t width); void ConvertYUVToRGB32Row_SSE(const uint8* yplane, const uint8* uplane, const uint8* vplane, uint8* rgbframe, - int width); + ptrdiff_t width); void ScaleYUVToRGB32Row_C(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void ScaleYUVToRGB32Row_MMX(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void ScaleYUVToRGB32Row_SSE(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void ScaleYUVToRGB32Row_SSE2_X64(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void LinearScaleYUVToRGB32Row_C(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void LinearScaleYUVToRGB32RowWithRange_C(const uint8* y_buf, const uint8* u_buf, @@ -136,22 +144,22 @@ void LinearScaleYUVToRGB32Row_MMX(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void LinearScaleYUVToRGB32Row_SSE(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); void LinearScaleYUVToRGB32Row_MMX_X64(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx); + ptrdiff_t width, + ptrdiff_t source_dx); } // extern "C" diff --git a/media/base/simd/convert_yuv_to_rgb_c.cc b/media/base/simd/convert_yuv_to_rgb_c.cc index db6e557..2849cac 100644 --- a/media/base/simd/convert_yuv_to_rgb_c.cc +++ b/media/base/simd/convert_yuv_to_rgb_c.cc @@ -45,7 +45,7 @@ void ConvertYUVToRGB32Row_C(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width) { + ptrdiff_t width) { for (int x = 0; x < width; x += 2) { uint8 u = u_buf[x >> 1]; uint8 v = v_buf[x >> 1]; @@ -67,8 +67,8 @@ void ScaleYUVToRGB32Row_C(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx) { + ptrdiff_t width, + ptrdiff_t source_dx) { int x = 0; for (int i = 0; i < width; i += 2) { int y = y_buf[x >> 16]; @@ -89,8 +89,8 @@ void LinearScaleYUVToRGB32Row_C(const uint8* y_buf, const uint8* u_buf, const uint8* v_buf, uint8* rgb_buf, - int width, - int source_dx) { + ptrdiff_t width, + ptrdiff_t source_dx) { // Avoid point-sampling for down-scaling by > 2:1. int source_x = 0; if (source_dx >= 0x20000) diff --git a/media/base/simd/convert_yuv_to_rgb_mmx.asm b/media/base/simd/convert_yuv_to_rgb_mmx.asm index e2a6617..39a4f75 100644 --- a/media/base/simd/convert_yuv_to_rgb_mmx.asm +++ b/media/base/simd/convert_yuv_to_rgb_mmx.asm @@ -17,6 +17,6 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width); +; ptrdiff_t width); %define SYMBOL ConvertYUVToRGB32Row_MMX %include "convert_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/convert_yuv_to_rgb_sse.asm b/media/base/simd/convert_yuv_to_rgb_sse.asm index 0fcec4f..8b3ee58 100644 --- a/media/base/simd/convert_yuv_to_rgb_sse.asm +++ b/media/base/simd/convert_yuv_to_rgb_sse.asm @@ -18,6 +18,6 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width); +; ptrdiff_t width); %define SYMBOL ConvertYUVToRGB32Row_SSE %include "convert_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/linear_scale_yuv_to_rgb_mmx.asm b/media/base/simd/linear_scale_yuv_to_rgb_mmx.asm index 4732aca..4041834 100644 --- a/media/base/simd/linear_scale_yuv_to_rgb_mmx.asm +++ b/media/base/simd/linear_scale_yuv_to_rgb_mmx.asm @@ -17,7 +17,7 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width, -; int source_dx); +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL LinearScaleYUVToRGB32Row_MMX %include "linear_scale_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/linear_scale_yuv_to_rgb_mmx_x64.asm b/media/base/simd/linear_scale_yuv_to_rgb_mmx_x64.asm index 2e43ba0..b6e8bb0 100644 --- a/media/base/simd/linear_scale_yuv_to_rgb_mmx_x64.asm +++ b/media/base/simd/linear_scale_yuv_to_rgb_mmx_x64.asm @@ -10,6 +10,12 @@ SECTION_TEXT CPU MMX +;void LinearScaleYUVToRGB32Row_MMX_X64(const uint8* y_buf, +; const uint8* u_buf, +; const uint8* v_buf, +; uint8* rgb_buf, +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL LinearScaleYUVToRGB32Row_MMX_X64 global mangle(SYMBOL) PRIVATE align function_align diff --git a/media/base/simd/linear_scale_yuv_to_rgb_sse.asm b/media/base/simd/linear_scale_yuv_to_rgb_sse.asm index 7bde0e7..0d6685f 100644 --- a/media/base/simd/linear_scale_yuv_to_rgb_sse.asm +++ b/media/base/simd/linear_scale_yuv_to_rgb_sse.asm @@ -17,7 +17,7 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width, -; int source_dx); +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL LinearScaleYUVToRGB32Row_SSE %include "linear_scale_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/scale_yuv_to_rgb_mmx.asm b/media/base/simd/scale_yuv_to_rgb_mmx.asm index 2b566d6..583b7cb 100644 --- a/media/base/simd/scale_yuv_to_rgb_mmx.asm +++ b/media/base/simd/scale_yuv_to_rgb_mmx.asm @@ -17,7 +17,7 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width, -; int source_dx); +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL ScaleYUVToRGB32Row_MMX %include "scale_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/scale_yuv_to_rgb_sse.asm b/media/base/simd/scale_yuv_to_rgb_sse.asm index a923c66..536ed18 100644 --- a/media/base/simd/scale_yuv_to_rgb_sse.asm +++ b/media/base/simd/scale_yuv_to_rgb_sse.asm @@ -17,7 +17,7 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width, -; int source_dx); +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL ScaleYUVToRGB32Row_SSE %include "scale_yuv_to_rgb_mmx.inc" diff --git a/media/base/simd/scale_yuv_to_rgb_sse2_x64.asm b/media/base/simd/scale_yuv_to_rgb_sse2_x64.asm index b9fab24..bdb80c1 100644 --- a/media/base/simd/scale_yuv_to_rgb_sse2_x64.asm +++ b/media/base/simd/scale_yuv_to_rgb_sse2_x64.asm @@ -14,8 +14,8 @@ ; const uint8* u_buf, ; const uint8* v_buf, ; uint8* rgb_buf, -; int width, -; int source_dx); +; ptrdiff_t width, +; ptrdiff_t source_dx); %define SYMBOL ScaleYUVToRGB32Row_SSE2_X64 global mangle(SYMBOL) PRIVATE |