Commit d4be6380 authored by wolenetz@chromium.org's avatar wolenetz@chromium.org

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
parent c9c89a15
...@@ -243,7 +243,7 @@ ...@@ -243,7 +243,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertARGBToYUVRow_SSSE3 %define SYMBOL ConvertARGBToYUVRow_SSSE3
%define PIXELSIZE 4 %define PIXELSIZE 4
...@@ -256,7 +256,7 @@ ...@@ -256,7 +256,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertRGBToYUVRow_SSSE3 %define SYMBOL ConvertRGBToYUVRow_SSSE3
%define PIXELSIZE 3 %define PIXELSIZE 3
...@@ -269,7 +269,7 @@ ...@@ -269,7 +269,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertARGBToYUVEven_SSSE3 %define SYMBOL ConvertARGBToYUVEven_SSSE3
%define PIXELSIZE 4 %define PIXELSIZE 4
...@@ -282,7 +282,7 @@ ...@@ -282,7 +282,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertARGBToYUVOdd_SSSE3 %define SYMBOL ConvertARGBToYUVOdd_SSSE3
%define PIXELSIZE 4 %define PIXELSIZE 4
...@@ -295,7 +295,7 @@ ...@@ -295,7 +295,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertRGBToYUVEven_SSSE3 %define SYMBOL ConvertRGBToYUVEven_SSSE3
%define PIXELSIZE 3 %define PIXELSIZE 3
...@@ -308,7 +308,7 @@ ...@@ -308,7 +308,7 @@
; uint8* y, ; uint8* y,
; uint8* u, ; uint8* u,
; uint8* v, ; uint8* v,
; int width); ; ptrdiff_t width);
; ;
%define SYMBOL ConvertRGBToYUVOdd_SSSE3 %define SYMBOL ConvertRGBToYUVOdd_SSSE3
%define PIXELSIZE 3 %define PIXELSIZE 3
......
...@@ -13,19 +13,25 @@ extern "C" { ...@@ -13,19 +13,25 @@ extern "C" {
// instructions so we can call them from C++ code. These functions are // instructions so we can call them from C++ code. These functions are
// implemented in "convert_rgb_to_yuv_ssse3.asm". // 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. // Convert a row of 24-bit RGB pixels to YV12 pixels.
void ConvertRGBToYUVRow_SSSE3(const uint8* rgb, void ConvertRGBToYUVRow_SSSE3(const uint8* rgb,
uint8* y, uint8* y,
uint8* u, uint8* u,
uint8* v, uint8* v,
int width); ptrdiff_t width);
// Convert a row of 32-bit RGB pixels to YV12 pixels. // Convert a row of 32-bit RGB pixels to YV12 pixels.
void ConvertARGBToYUVRow_SSSE3(const uint8* argb, void ConvertARGBToYUVRow_SSSE3(const uint8* argb,
uint8* y, uint8* y,
uint8* u, uint8* u,
uint8* v, uint8* v,
int width); ptrdiff_t width);
#ifdef __cplusplus #ifdef __cplusplus
} }
......
...@@ -59,70 +59,78 @@ void ConvertYUVToRGB32_MMX(const uint8* yplane, ...@@ -59,70 +59,78 @@ void ConvertYUVToRGB32_MMX(const uint8* yplane,
// Assembly functions are declared without namespace. // Assembly functions are declared without namespace.
extern "C" { 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*, typedef void (*ConvertYUVToRGB32RowProc)(const uint8*,
const uint8*, const uint8*,
const uint8*, const uint8*,
uint8*, uint8*,
int); ptrdiff_t);
typedef void (*ScaleYUVToRGB32RowProc)(const uint8*, typedef void (*ScaleYUVToRGB32RowProc)(const uint8*,
const uint8*, const uint8*,
const uint8*, const uint8*,
uint8*, uint8*,
int, ptrdiff_t,
int); ptrdiff_t);
void ConvertYUVToRGB32Row_C(const uint8* yplane, void ConvertYUVToRGB32Row_C(const uint8* yplane,
const uint8* uplane, const uint8* uplane,
const uint8* vplane, const uint8* vplane,
uint8* rgbframe, uint8* rgbframe,
int width); ptrdiff_t width);
void ConvertYUVToRGB32Row_MMX(const uint8* yplane, void ConvertYUVToRGB32Row_MMX(const uint8* yplane,
const uint8* uplane, const uint8* uplane,
const uint8* vplane, const uint8* vplane,
uint8* rgbframe, uint8* rgbframe,
int width); ptrdiff_t width);
void ConvertYUVToRGB32Row_SSE(const uint8* yplane, void ConvertYUVToRGB32Row_SSE(const uint8* yplane,
const uint8* uplane, const uint8* uplane,
const uint8* vplane, const uint8* vplane,
uint8* rgbframe, uint8* rgbframe,
int width); ptrdiff_t width);
void ScaleYUVToRGB32Row_C(const uint8* y_buf, void ScaleYUVToRGB32Row_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void ScaleYUVToRGB32Row_MMX(const uint8* y_buf, void ScaleYUVToRGB32Row_MMX(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void ScaleYUVToRGB32Row_SSE(const uint8* y_buf, void ScaleYUVToRGB32Row_SSE(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void ScaleYUVToRGB32Row_SSE2_X64(const uint8* y_buf, void ScaleYUVToRGB32Row_SSE2_X64(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void LinearScaleYUVToRGB32Row_C(const uint8* y_buf, void LinearScaleYUVToRGB32Row_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void LinearScaleYUVToRGB32RowWithRange_C(const uint8* y_buf, void LinearScaleYUVToRGB32RowWithRange_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
...@@ -136,22 +144,22 @@ void LinearScaleYUVToRGB32Row_MMX(const uint8* y_buf, ...@@ -136,22 +144,22 @@ void LinearScaleYUVToRGB32Row_MMX(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void LinearScaleYUVToRGB32Row_SSE(const uint8* y_buf, void LinearScaleYUVToRGB32Row_SSE(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
void LinearScaleYUVToRGB32Row_MMX_X64(const uint8* y_buf, void LinearScaleYUVToRGB32Row_MMX_X64(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx); ptrdiff_t source_dx);
} // extern "C" } // extern "C"
......
...@@ -45,7 +45,7 @@ void ConvertYUVToRGB32Row_C(const uint8* y_buf, ...@@ -45,7 +45,7 @@ void ConvertYUVToRGB32Row_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width) { ptrdiff_t width) {
for (int x = 0; x < width; x += 2) { for (int x = 0; x < width; x += 2) {
uint8 u = u_buf[x >> 1]; uint8 u = u_buf[x >> 1];
uint8 v = v_buf[x >> 1]; uint8 v = v_buf[x >> 1];
...@@ -67,8 +67,8 @@ void ScaleYUVToRGB32Row_C(const uint8* y_buf, ...@@ -67,8 +67,8 @@ void ScaleYUVToRGB32Row_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx) { ptrdiff_t source_dx) {
int x = 0; int x = 0;
for (int i = 0; i < width; i += 2) { for (int i = 0; i < width; i += 2) {
int y = y_buf[x >> 16]; int y = y_buf[x >> 16];
...@@ -89,8 +89,8 @@ void LinearScaleYUVToRGB32Row_C(const uint8* y_buf, ...@@ -89,8 +89,8 @@ void LinearScaleYUVToRGB32Row_C(const uint8* y_buf,
const uint8* u_buf, const uint8* u_buf,
const uint8* v_buf, const uint8* v_buf,
uint8* rgb_buf, uint8* rgb_buf,
int width, ptrdiff_t width,
int source_dx) { ptrdiff_t source_dx) {
// Avoid point-sampling for down-scaling by > 2:1. // Avoid point-sampling for down-scaling by > 2:1.
int source_x = 0; int source_x = 0;
if (source_dx >= 0x20000) if (source_dx >= 0x20000)
......
...@@ -17,6 +17,6 @@ ...@@ -17,6 +17,6 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width); ; ptrdiff_t width);
%define SYMBOL ConvertYUVToRGB32Row_MMX %define SYMBOL ConvertYUVToRGB32Row_MMX
%include "convert_yuv_to_rgb_mmx.inc" %include "convert_yuv_to_rgb_mmx.inc"
...@@ -18,6 +18,6 @@ ...@@ -18,6 +18,6 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width); ; ptrdiff_t width);
%define SYMBOL ConvertYUVToRGB32Row_SSE %define SYMBOL ConvertYUVToRGB32Row_SSE
%include "convert_yuv_to_rgb_mmx.inc" %include "convert_yuv_to_rgb_mmx.inc"
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width, ; ptrdiff_t width,
; int source_dx); ; ptrdiff_t source_dx);
%define SYMBOL LinearScaleYUVToRGB32Row_MMX %define SYMBOL LinearScaleYUVToRGB32Row_MMX
%include "linear_scale_yuv_to_rgb_mmx.inc" %include "linear_scale_yuv_to_rgb_mmx.inc"
...@@ -10,6 +10,12 @@ ...@@ -10,6 +10,12 @@
SECTION_TEXT SECTION_TEXT
CPU MMX 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 %define SYMBOL LinearScaleYUVToRGB32Row_MMX_X64
global mangle(SYMBOL) PRIVATE global mangle(SYMBOL) PRIVATE
align function_align align function_align
......
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width, ; ptrdiff_t width,
; int source_dx); ; ptrdiff_t source_dx);
%define SYMBOL LinearScaleYUVToRGB32Row_SSE %define SYMBOL LinearScaleYUVToRGB32Row_SSE
%include "linear_scale_yuv_to_rgb_mmx.inc" %include "linear_scale_yuv_to_rgb_mmx.inc"
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width, ; ptrdiff_t width,
; int source_dx); ; ptrdiff_t source_dx);
%define SYMBOL ScaleYUVToRGB32Row_MMX %define SYMBOL ScaleYUVToRGB32Row_MMX
%include "scale_yuv_to_rgb_mmx.inc" %include "scale_yuv_to_rgb_mmx.inc"
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width, ; ptrdiff_t width,
; int source_dx); ; ptrdiff_t source_dx);
%define SYMBOL ScaleYUVToRGB32Row_SSE %define SYMBOL ScaleYUVToRGB32Row_SSE
%include "scale_yuv_to_rgb_mmx.inc" %include "scale_yuv_to_rgb_mmx.inc"
...@@ -14,8 +14,8 @@ ...@@ -14,8 +14,8 @@
; const uint8* u_buf, ; const uint8* u_buf,
; const uint8* v_buf, ; const uint8* v_buf,
; uint8* rgb_buf, ; uint8* rgb_buf,
; int width, ; ptrdiff_t width,
; int source_dx); ; ptrdiff_t source_dx);
%define SYMBOL ScaleYUVToRGB32Row_SSE2_X64 %define SYMBOL ScaleYUVToRGB32Row_SSE2_X64
global mangle(SYMBOL) PRIVATE global mangle(SYMBOL) PRIVATE
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment