Skip to content

Commit

Permalink
Fixed bug 3857 - SDL_ConvertPixels misses YUV conversions
Browse files Browse the repository at this point in the history
Sylvain

Few issues with YUV on SDL2 when using odd dimensions, and missing conversions from/back to YUV formats.

1) The big part is that SDL_ConvertPixels() does not convert to/from YUV in most cases. This now works with any format and also with odd dimensions,
  by adding two internal functions SDL_ConvertPixels_YUV_to_ARGB8888 and SDL_ConvertPixels_ARGB8888_to_YUV (could it be XRGB888 ?).
  The target format is hard coded to ARGB888 (which is the default in the internal of the software renderer).
  In case of different YUV conversion, it will do an intermediate conversion to a ARGB8888 buffer.

  SDL_ConvertPixels_YUV_to_ARGB8888 is somehow redundant with all the "Color*Dither*Mod*".
  But it allows some completeness of SDL_ConvertPixels to handle all YUV format.
  It also works with odd dimensions.

  Moreover, I did some benchmark(SDL_ConvertPixel vs Color32DitherYV12Mod1X and Color32DitherYUY2Mod1X).
  gcc-6.3 and clang-4.0. gcc performs better than clang. And, with gcc, SDL_ConvertPixels() performs better (20%) than the two C function Color32Dither*().
  For instance, to convert 10 times a 3888x2592 image, it takes ~195 ms with SDL_ConvertPixels and ~235 ms with Color32Dither*().
  Especially because of gcc vectorize feature that optimises all conversion loops (-ftree-loop-vectorize).

  Nb: I put no image pitch for the YUV buffers. because it complexify a little bit the code and the API :
  There would be some ambiguity when setting the pitch exactly to image width:
  would it a be pitch of image width (for luma and chroma). or just contiguous data ? (could set pitch=0 for the later).


2) Small issues with odd dimensions:
  If width "w" is odd, luma plane width is still "w" whereas chroma planes will be "(w + 1)/2". Almost the same for odd h.
  Solution is to strategically substitute "w" by "(w+1)/2" at the good places ...

- In the repository, SDL_ConvertPixels() handles YUV only if yuv source format is exactly the same as YUV destination format.
  It basically does a memcpy of pixels, but it's done incorrectly when width or height is odd (wrong size of chroma planes). This is fixed.

- SDL Renderers don't support odd width/height for YUV textures.
  This is fixed for software, opengl, opengles2. (opengles 1 does not support it and fallback to software rendering).
  This is *not* fixed for D3D and D3D11 ... (and others, psp ?)
  Only *two* Dither function are fixed ... not sure if others are really used.

- This is not possible to create a NV12/NV12 texture with the software renderer, whereas other renderers allow it.
  This is fixed, by using SDL_ConvertPixels underneath.

- It was not possible to SDL_UpdateTexture() of format NV12/NV21 with the software renderer. this is fixed.

Here's also two testcases:
- that do all combination of conversion.
- to test partial UpdateTexture
  • Loading branch information
slouken committed Oct 6, 2017
1 parent 827e985 commit e9652b1
Show file tree
Hide file tree
Showing 4 changed files with 767 additions and 97 deletions.
185 changes: 150 additions & 35 deletions src/render/SDL_yuv_sw.c
Expand Up @@ -265,15 +265,26 @@ Color32DitherYV12Mod1X(int *colortab, Uint32 * rgb_2_pix,
int cr_r;
int crb_g;
int cb_b;
int cols_2 = cols / 2;
int cols_2 = (cols + 1) / 2;
/* not even dimensions */
int skip_last_col = 0;
int skip_last_row = 0;

if ( (cols & 0x1) ) {
skip_last_col = 1;
}

if ( (rows & 0x1) ) {
skip_last_row = 1;
}

row1 = (unsigned int *) out;
row2 = row1 + cols + mod;
lum2 = lum + cols;

mod += cols + mod;

y = rows / 2;
y = (rows + 1) / 2;
while (y--) {
x = cols_2;
while (x--) {
Expand All @@ -290,20 +301,27 @@ Color32DitherYV12Mod1X(int *colortab, Uint32 * rgb_2_pix,
*row1++ = (rgb_2_pix[L + cr_r] |
rgb_2_pix[L + crb_g] | rgb_2_pix[L + cb_b]);

if (!(x == 0 && skip_last_col)) {
L = *lum++;
*row1++ = (rgb_2_pix[L + cr_r] |
rgb_2_pix[L + crb_g] | rgb_2_pix[L + cb_b]);
} /* skip col */


if (!(y == 0 && skip_last_row)) {

/* Now, do second row. */

L = *lum2++;
*row2++ = (rgb_2_pix[L + cr_r] |
rgb_2_pix[L + crb_g] | rgb_2_pix[L + cb_b]);

if (!(x == 1 && skip_last_col)) {
L = *lum2++;
*row2++ = (rgb_2_pix[L + cr_r] |
rgb_2_pix[L + crb_g] | rgb_2_pix[L + cb_b]);
} /* skip col */
} /* skip row */
}

/*
Expand Down Expand Up @@ -670,7 +688,12 @@ Color32DitherYUY2Mod1X(int *colortab, Uint32 * rgb_2_pix,
int cr_r;
int crb_g;
int cb_b;
int cols_2 = cols / 2;
int cols_2 = (cols + 1) / 2;
/* not even dimensions */
int skip_last_col = 0;
if ( (cols & 0x1) ) {
skip_last_col = 1;
}

row = (unsigned int *) out;
y = rows;
Expand All @@ -693,9 +716,11 @@ Color32DitherYUY2Mod1X(int *colortab, Uint32 * rgb_2_pix,

L = *lum;
lum += 2;

if (!(x == 0 && skip_last_col)) {
*row++ = (rgb_2_pix[L + cr_r] |
rgb_2_pix[L + crb_g] | rgb_2_pix[L + cb_b]);

} /* skip col */

}
row += mod;
Expand Down Expand Up @@ -1022,6 +1047,13 @@ SDL_SW_SetupYUVDisplay(SDL_SW_YUVTexture * swdata, Uint32 target_format)
swdata->Display2X = Color32DitherYUY2Mod2X;
}
break;
case SDL_PIXELFORMAT_NV21:
case SDL_PIXELFORMAT_NV12:
/* no Display{1,2}X function */
swdata->Display1X = NULL;
swdata->Display2X = NULL;
break;

default:
/* We should never get here (caught above) */
break;
Expand Down Expand Up @@ -1049,6 +1081,8 @@ SDL_SW_CreateYUVTexture(Uint32 format, int w, int h)
case SDL_PIXELFORMAT_YUY2:
case SDL_PIXELFORMAT_UYVY:
case SDL_PIXELFORMAT_YVYU:
case SDL_PIXELFORMAT_NV12:
case SDL_PIXELFORMAT_NV21:
break;
default:
SDL_SetError("Unsupported YUV format");
Expand All @@ -1065,7 +1099,35 @@ SDL_SW_CreateYUVTexture(Uint32 format, int w, int h)
swdata->target_format = SDL_PIXELFORMAT_UNKNOWN;
swdata->w = w;
swdata->h = h;
swdata->pixels = (Uint8 *) SDL_malloc(w * h * 2);
{
const int sz_plane = w * h;
const int sz_plane_chroma = ((w + 1) / 2) * ((h + 1) / 2);
const int sz_plane_packed = ((w + 1) / 2) * h;
int dst_size = 0;
switch(format)
{
case SDL_PIXELFORMAT_YV12: /**< Planar mode: Y + V + U (3 planes) */
case SDL_PIXELFORMAT_IYUV: /**< Planar mode: Y + U + V (3 planes) */
dst_size = sz_plane + sz_plane_chroma + sz_plane_chroma;
break;

case SDL_PIXELFORMAT_YUY2: /**< Packed mode: Y0+U0+Y1+V0 (1 plane) */
case SDL_PIXELFORMAT_UYVY: /**< Packed mode: U0+Y0+V0+Y1 (1 plane) */
case SDL_PIXELFORMAT_YVYU: /**< Packed mode: Y0+V0+Y1+U0 (1 plane) */
dst_size = 4 * sz_plane_packed;
break;

case SDL_PIXELFORMAT_NV12: /**< Planar mode: Y + U/V interleaved (2 planes) */
case SDL_PIXELFORMAT_NV21: /**< Planar mode: Y + V/U interleaved (2 planes) */
dst_size = sz_plane + sz_plane_chroma + sz_plane_chroma;
break;

default:
SDL_assert(0 && "We should never get here (caught above)");
break;
}
swdata->pixels = (Uint8 *) SDL_malloc(dst_size);
}
swdata->colortab = (int *) SDL_malloc(4 * 256 * sizeof(int));
swdata->rgb_2_pix = (Uint32 *) SDL_malloc(3 * 768 * sizeof(Uint32));
if (!swdata->pixels || !swdata->colortab || !swdata->rgb_2_pix) {
Expand Down Expand Up @@ -1095,18 +1157,27 @@ SDL_SW_CreateYUVTexture(Uint32 format, int w, int h)
case SDL_PIXELFORMAT_YV12:
case SDL_PIXELFORMAT_IYUV:
swdata->pitches[0] = w;
swdata->pitches[1] = swdata->pitches[0] / 2;
swdata->pitches[2] = swdata->pitches[0] / 2;
swdata->pitches[1] = (swdata->pitches[0] + 1) / 2;
swdata->pitches[2] = (swdata->pitches[0] + 1) / 2;
swdata->planes[0] = swdata->pixels;
swdata->planes[1] = swdata->planes[0] + swdata->pitches[0] * h;
swdata->planes[2] = swdata->planes[1] + swdata->pitches[1] * h / 2;
swdata->planes[2] = swdata->planes[1] + swdata->pitches[1] * ((h + 1) / 2);
break;
case SDL_PIXELFORMAT_YUY2:
case SDL_PIXELFORMAT_UYVY:
case SDL_PIXELFORMAT_YVYU:
swdata->pitches[0] = w * 2;
swdata->pitches[0] = ((w + 1) / 2) * 4;
swdata->planes[0] = swdata->pixels;
break;

case SDL_PIXELFORMAT_NV12:
case SDL_PIXELFORMAT_NV21:
swdata->pitches[0] = w;
swdata->pitches[1] = 2 * ((swdata->pitches[0] + 1) / 2);
swdata->planes[0] = swdata->pixels;
swdata->planes[1] = swdata->planes[0] + swdata->pitches[0] * h;
break;

default:
SDL_assert(0 && "We should never get here (caught above)");
break;
Expand Down Expand Up @@ -1135,7 +1206,7 @@ SDL_SW_UpdateYUVTexture(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
if (rect->x == 0 && rect->y == 0 &&
rect->w == swdata->w && rect->h == swdata->h) {
SDL_memcpy(swdata->pixels, pixels,
(swdata->h * swdata->w) + (swdata->h * swdata->w) / 2);
(swdata->h * swdata->w) + 2* ((swdata->h + 1) /2) * ((swdata->w + 1) / 2));
} else {
Uint8 *src, *dst;
int row;
Expand All @@ -1150,28 +1221,28 @@ SDL_SW_UpdateYUVTexture(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
src += pitch;
dst += swdata->w;
}

/* Copy the next plane */
src = (Uint8 *) pixels + rect->h * pitch;
dst = swdata->pixels + swdata->h * swdata->w;
dst += rect->y/2 * swdata->w/2 + rect->x/2;
length = rect->w / 2;
for (row = 0; row < rect->h/2; ++row) {
dst += rect->y/2 * ((swdata->w + 1) / 2) + rect->x/2;
length = (rect->w + 1) / 2;
for (row = 0; row < (rect->h + 1)/2; ++row) {
SDL_memcpy(dst, src, length);
src += pitch/2;
dst += swdata->w/2;
src += (pitch + 1)/2;
dst += (swdata->w + 1)/2;
}

/* Copy the next plane */
src = (Uint8 *) pixels + rect->h * pitch + (rect->h * pitch) / 4;
src = (Uint8 *) pixels + rect->h * pitch + ((rect->h + 1) / 2) * ((pitch + 1) / 2);
dst = swdata->pixels + swdata->h * swdata->w +
(swdata->h * swdata->w) / 4;
dst += rect->y/2 * swdata->w/2 + rect->x/2;
length = rect->w / 2;
for (row = 0; row < rect->h/2; ++row) {
((swdata->h + 1)/2) * ((swdata->w+1) / 2);
dst += rect->y/2 * ((swdata->w + 1)/2) + rect->x/2;
length = (rect->w + 1) / 2;
for (row = 0; row < (rect->h + 1)/2; ++row) {
SDL_memcpy(dst, src, length);
src += pitch/2;
dst += swdata->w/2;
src += (pitch + 1)/2;
dst += (swdata->w + 1)/2;
}
}
break;
Expand All @@ -1187,14 +1258,50 @@ SDL_SW_UpdateYUVTexture(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
dst =
swdata->planes[0] + rect->y * swdata->pitches[0] +
rect->x * 2;
length = rect->w * 2;
length = 4 * ((rect->w + 1) / 2);
for (row = 0; row < rect->h; ++row) {
SDL_memcpy(dst, src, length);
src += pitch;
dst += swdata->pitches[0];
}
}
break;
case SDL_PIXELFORMAT_NV12:
case SDL_PIXELFORMAT_NV21:
{
if (rect->x == 0 && rect->y == 0 && rect->w == swdata->w && rect->h == swdata->h) {
SDL_memcpy(swdata->pixels, pixels,
(swdata->h * swdata->w) + 2* ((swdata->h + 1) /2) * ((swdata->w + 1) / 2));
} else {

Uint8 *src, *dst;
int row;
size_t length;

/* Copy the Y plane */
src = (Uint8 *) pixels;
dst = swdata->pixels + rect->y * swdata->w + rect->x;
length = rect->w;
for (row = 0; row < rect->h; ++row) {
SDL_memcpy(dst, src, length);
src += pitch;
dst += swdata->w;
}

/* Copy the next plane */
src = (Uint8 *) pixels + rect->h * pitch;
dst = swdata->pixels + swdata->h * swdata->w;
dst += 2 * ((rect->y + 1)/2) * ((swdata->w + 1) / 2) + 2 * (rect->x/2);
length = 2 * ((rect->w + 1) / 2);
for (row = 0; row < (rect->h + 1)/2; ++row) {
SDL_memcpy(dst, src, length);
src += 2 * ((pitch + 1)/2);
dst += 2 * ((swdata->w + 1)/2);
}
}
}
break;

}
return 0;
}
Expand Down Expand Up @@ -1226,14 +1333,14 @@ SDL_SW_UpdateYUVTexturePlanar(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
dst = swdata->pixels + swdata->h * swdata->w;
} else {
dst = swdata->pixels + swdata->h * swdata->w +
(swdata->h * swdata->w) / 4;
((swdata->h + 1) / 2) * ((swdata->w + 1) / 2);
}
dst += rect->y/2 * swdata->w/2 + rect->x/2;
length = rect->w / 2;
for (row = 0; row < rect->h/2; ++row) {
dst += rect->y/2 * ((swdata->w + 1)/2) + rect->x/2;
length = (rect->w + 1) / 2;
for (row = 0; row < (rect->h + 1)/2; ++row) {
SDL_memcpy(dst, src, length);
src += Upitch;
dst += swdata->w/2;
dst += (swdata->w + 1)/2;
}

/* Copy the V plane */
Expand All @@ -1242,14 +1349,14 @@ SDL_SW_UpdateYUVTexturePlanar(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
dst = swdata->pixels + swdata->h * swdata->w;
} else {
dst = swdata->pixels + swdata->h * swdata->w +
(swdata->h * swdata->w) / 4;
((swdata->h + 1) / 2) * ((swdata->w + 1) / 2);
}
dst += rect->y/2 * swdata->w/2 + rect->x/2;
length = rect->w / 2;
for (row = 0; row < rect->h/2; ++row) {
dst += rect->y/2 * ((swdata->w + 1)/2) + rect->x/2;
length = (rect->w + 1) / 2;
for (row = 0; row < (rect->h + 1)/2; ++row) {
SDL_memcpy(dst, src, length);
src += Vpitch;
dst += swdata->w/2;
dst += (swdata->w + 1)/2;
}
return 0;
}
Expand All @@ -1261,11 +1368,13 @@ SDL_SW_LockYUVTexture(SDL_SW_YUVTexture * swdata, const SDL_Rect * rect,
switch (swdata->format) {
case SDL_PIXELFORMAT_YV12:
case SDL_PIXELFORMAT_IYUV:
case SDL_PIXELFORMAT_NV12:
case SDL_PIXELFORMAT_NV21:
if (rect
&& (rect->x != 0 || rect->y != 0 || rect->w != swdata->w
|| rect->h != swdata->h)) {
return SDL_SetError
("YV12 and IYUV textures only support full surface locks");
("YV12, IYUV, NV12, NV21 textures only support full surface locks");
}
break;
}
Expand Down Expand Up @@ -1383,6 +1492,12 @@ SDL_SW_CopyYUVToRGB(SDL_SW_YUVTexture * swdata, const SDL_Rect * srcrect,
Cr = lum + 1;
Cb = lum + 3;
break;
case SDL_PIXELFORMAT_NV12:
case SDL_PIXELFORMAT_NV21:
return SDL_ConvertPixels(swdata->w, swdata->h,
swdata->format, swdata->planes[0], swdata->pitches[0],
target_format, pixels, pitch);
break;
default:
return SDL_SetError("Unsupported YUV format in copy");
}
Expand Down

0 comments on commit e9652b1

Please sign in to comment.