We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/d9a2fa86cd97

Seems broken, in GLES2_TexSubImage2D:

- src = (Uint8 *)data;
+ src = (Uint8 *)pixels;

Fixed, thanks! https://hg.libsdl.org/SDL/rev/d3c44f989e4d

Sylvain's fix corrected rendering contiguous (unpadded) YUV (YV12). This had just broke. I was looking into this as I figured I was doing wrong on my end.

Unfortunately, it still doesn't seem to be working for padded, non-contiguous YUV. I will attach a screenshot of a test program I have been using to investigate this bug.

When I render the unpadded, contiguous YUV to a RGB24 target texture, the image is greyscale. Again, not sure if this is related or not.

I will also attach my test program. It requires FFMPEG. It simply loads a video file and offers various rendering methods, see --help. Also, you can click (or tap if running it on Android) to cycle between the various methods whilst the frames are being rendered. I use SDL_Log() for reporting, so you will need a console.

I'm afraid I won't be much help on the OpenGL ES 2.0 side of the code. However, I am willing to test and perhaps my test program will help you guys.

When I run my test program on my Android (4.0.4 - Samsung Galaxy Tab 10.1), I get a SIGSEGV fatal error when I try to render the unpadded, non-contiguous YUV. I have yet to master debugging on Android (outside of using plenty of SDL_Log() calls), but I see in the CatLog that the error happens with something to do with SDL_memcpy_REAL. Not sure if that helps. On the desktop, my test program doesn't crash. I've tested my test program on Android using SDL2 from Mecurial (26 May 2014) and it doesn't crash and valgrind does not report the errors below.

Valgrind (via valkyrie) reports the following for d3c44f989e4d:

==22856== Invalid read of size 8
==22856==    at 0x4C2DF3A: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4EA31E7: GLES2_UpdateTextureYUV (SDL_render_gles2.c:604)
==22856==    by 0x4E9367F: SDL_UpdateYUVTexture_REAL (SDL_render.c:916)
==22856==    by 0x402E5D: main (yuvtest.c:589)
==22856==  Address 0x173febc8 is 8 bytes before a block of size 172,800 alloc'd
==22856==    at 0x4C2C360: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4EA31A3: GLES2_UpdateTextureYUV (SDL_render_gles2.c:597)
==22856==    by 0x4E9367F: SDL_UpdateYUVTexture_REAL (SDL_render.c:916)
==22856==    by 0x402E5D: main (yuvtest.c:589)
==22856==
==22856== Invalid read of size 8
==22856==    at 0x4C2DF28: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4EA31E7: GLES2_UpdateTextureYUV (SDL_render_gles2.c:604)
==22856==    by 0x4E9367F: SDL_UpdateYUVTexture_REAL (SDL_render.c:916)
==22856==    by 0x402E5D: main (yuvtest.c:589)
==22856==  Address 0x173febb0 is not stack'd, malloc'd or (recently) free'd
==22856==
==22856== Invalid read of size 8
==22856==    at 0x4C2DE46: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4EA31E7: GLES2_UpdateTextureYUV (SDL_render_gles2.c:604)
==22856==    by 0x4E9367F: SDL_UpdateYUVTexture_REAL (SDL_render.c:916)
==22856==    by 0x402E5D: main (yuvtest.c:589)
==22856==  Address 0x173feb90 is 0 bytes after a block of size 43,200 alloc'd
==22856==    at 0x4C2C360: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4024FA: pack_unpadded (yuvtest.c:374)
==22856==    by 0x402DFC: main (yuvtest.c:582)
==22856==
==22856== Invalid read of size 8
==22856==    at 0x4C2DE38: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==22856==    by 0x4EA31E7: GLES2_UpdateTextureYUV (SDL_render_gles2.c:604)
==22856==    by 0x4E9367F: SDL_UpdateYUVTexture_REAL (SDL_render.c:916)
==22856==    by 0x402E5D: main (yuvtest.c:589)
==22856==  Address 0x173feba8 is not stack'd, malloc'd or (recently) free'd

Created attachment 1703 [details]
Result of rendering a padded, non-contiguous YUV image on a YV12 texture.

Created attachment 1704 [details]
Test program to testing OpenGL ES 2.0 YUV rendering

This requires FFMPEG. It loads a video file and offers various rendering methods.

This forces the "opengles2" renderer.

Running without any args will render padded (mostly likely), non-contiguous YUV.

Click (or tap if running it on Android) to cycle between the various rendering methods whilst the frames are being rendered. SDL_Log() is used for reporting, so a console will be needed.

If the '-cont' switch is used at start up, the image is bright green. However, if run without args and the window is clicked, contiguous YUV rendering looks fine.

To compile (assumes that sdl2-config is the one produced from Mercurial):

gcc -o yuvtest yuvtest.c $(sdl2-config --cflags) -std=c99 $(sdl2-config --libs) -lavcodec -lavformat -lavutil

Usage:
Usage: yuvtest [-cont|-unpad] [-rgb] FILE
       Click to cycle between rendering methods.
-cont  Force unpadded contiguous YUV buffer
-unpad Force unpadded, non-contiguous YUV buffers
-rgb   Copy YUV texture to RGB24 texture. Render the RGB24 texture.

Alvin, 

Not really familiar with that, I just reported the fix, because after updating the trunk it wasn't working for me.

Just an idea, about the YUV: I may be totally wrong, but the data you provide wouldn't be encoded / interleaved inside the width ?

so, instead of having : 

          for (y = 0; y < height; ++y)
          {
              SDL_memcpy(src, pixels, src_pitch);
              src += src_pitch;
              pixels = (Uint8 *)pixels + pitch;
          }

you would also have something a pixel-level : 

          for (y = 0; y < height; ++y)
          {
              src += init_inc;

              for (x = 0; x < width; ++x)
              {
                 *pixels = *src;

                 src += src_inc;
                 pixels = (Uint8 *)pixels + pitch;
              }

              src += (src_pitch - width);
          }

I tried the yuvtest (need to replace av_frame_alloc/free by malloc/free).
and there are frames displayed/decoded, but looks like some pb of shift and pitch.

Alvin, I looked again, it seems to me your last call to "GLES2_TexSubImage2D" should done with (Yplane, Ypitch).

Created attachment 1705 [details]
Patch to fix typo in GLES2_UpdateTextureYUV

With this patch, I confirm that padded, non-contiguous YUV renders correctly using the opengles2 renderer. Additionally, contiguous YUV, unpadded non-contiguous YUV have also been successfully tested.

Tested on a desktop (forced opengles2 renderer) as well as on an Android tablet (Samsung Galaxy Tab 10.1 - 4.0.4)

Credit goes to Sylvain for catching the typo!

Hey, thanks but not sure if this can be said to be resolved

"-unpad" is working
But, with the option "-cont" (Force unpadded contiguous YUV buffer), there is still a "green overlay" with the video I that I tried.

I forgot about that case! I even mentioned it in Comment 6:

"If the '-cont' switch is used at start up, the image is bright green. However, if run without args and the window is clicked, contiguous YUV rendering looks fine."

I ran yuvtest and clicked through all the modes. I forgot about re-testing with starting up with '-cont'.

I was just debugging the same issue with the Android camera (green picture).

So far I managed to work around it by calling SDL_UpdateYUVTexture() and using pointer arithmetic into the contiguous YUV buffer. I have been using SDL_UpdateTexture(), but that renders a green image. yuvtest uses SDL_UpdateTexture() for '-cont'.

It seems to me that SDL_UpdateYUVTexture() initialises something that SDL_UpdateTexture() does not? For instance, run with '-cont' and the video will appear green. Click on the video twice. When it cycles back to rendering unpadded contiguous YUV, it will display correctly.

So, "contiguous" mode has a green transparent overlay.
This is the call to SDL_UpdateTexture (and GLES2_UpdateTexture).
So where texture is created as a YV12 (and also queried correctly as YV12).

During rendering, when we toggle to a non-contiguous mode, SDL_UpdateTextureYUV is called, then GLES2_UpdateTextureYUV is called. The rendering is fine at that time.

And when we get back to the "contiguous" mode. The green has disappeared.
But, if I wait for more Frames. I see the image in background.
Like if Y plane moving, but UVplanes were fixed (initialized during GLES2_UpdateTextureYUV).
But we are in contiguous mode ! which is strange, because we always give YUV up to date.

-> It seems to me that contiguous/SDL_UpdateTexture mode is only using Y. never UV. It calls GLES2_UpdateTexture with a W*H*bpp equals to Y frame. missing some room for UV.

If I remove the Yplane (replace by zero's) from the image, I only see green.
If I put the UV at the place of Y, I see the two small pictures. The Datas provided seems to be correct. UV seems discarded.

I don't know enough SDL to help you ...

(In reply to Sylvain from comment #12)
> It seems to me that contiguous/SDL_UpdateTexture mode is only using Y.
> never UV. It calls GLES2_UpdateTexture with a W*H*bpp equals to Y frame.
> missing some room for UV.

I think that is a good assessment. It would explain why when the YUV textured is render copied to a RGB24 texture, the resulting image is greyscale.

Ok, I found out : GLES2_UpdateTexture is just not handling the YUV, I will attach a patch.

Maybe as to be reviewed ...

Created attachment 1707 [details]
patch for contiguous YUV

patch for contiguous YUV

Comment on attachment 1707 [details]
patch for contiguous YUV


>diff -r d9a2fa86cd97 src/render/opengles2/SDL_render_gles2.c
>--- a/src/render/opengles2/SDL_render_gles2.c	Sat Jun 21 12:38:46 2014 -0700
>+++ b/src/render/opengles2/SDL_render_gles2.c	Tue Jun 24 22:17:43 2014 +0200
>@@ -590,7 +590,7 @@
> 
>     /* Reformat the texture data into a tightly packed array */
>     src_pitch = width * bpp;
>-    src = (Uint8 *)data;
>+    src = (Uint8 *)pixels;
>     if (pitch != src_pitch) {
>         blob = (Uint8 *)SDL_malloc(src_pitch * height);
>         if (!blob) {
>@@ -637,6 +637,41 @@
>                     tdata->pixel_type,
>                     pixels, pitch, SDL_BYTESPERPIXEL(texture->format));
> 
>+    if (tdata->yuv) {
>+
>+        /* Skip to the correct offset into the next texture */
>+        pixels = (const void*)((const Uint8*)pixels + rect->h * pitch);
>+        if (texture->format == SDL_PIXELFORMAT_YV12) {
>+            data->glBindTexture(tdata->texture_type, tdata->texture_v);
>+        } else {
>+            data->glBindTexture(tdata->texture_type, tdata->texture_u);
>+        }
>+        GLES2_TexSubImage2D(data, tdata->texture_type,
>+                rect->x / 2,
>+                rect->y / 2,
>+                rect->w / 2,
>+                rect->h / 2,
>+                tdata->pixel_format,
>+                tdata->pixel_type,
>+                pixels, pitch / 2, 1);
>+
>+        /* Skip to the correct offset into the next texture */
>+        pixels = (const void*)((const Uint8*)pixels + (rect->h * pitch)/4);
>+        if (texture->format == SDL_PIXELFORMAT_YV12) {
>+            data->glBindTexture(tdata->texture_type, tdata->texture_u);
>+        } else {
>+            data->glBindTexture(tdata->texture_type, tdata->texture_v);
>+        }
>+        GLES2_TexSubImage2D(data, tdata->texture_type,
>+                rect->x / 2,
>+                rect->y / 2,
>+                rect->w / 2,
>+                rect->h / 2,
>+                tdata->pixel_format,
>+                tdata->pixel_type,
>+                pixels, pitch / 2, 1);
>+    }
>+
>     return GL_CheckError("glTexSubImage2D()", renderer);
> }
> 
>@@ -684,7 +719,7 @@
>                     rect->h,
>                     tdata->pixel_format,
>                     tdata->pixel_type,
>-                    Vplane, Vpitch, 1);
>+                    Yplane, Ypitch, 1);
> 
>     return GL_CheckError("glTexSubImage2D()", renderer);
> }

sorry, just ignore the "configure" part of the patch.

Created attachment 1708 [details]
Another patch for GLES2_UpdateTexture

This patch does not actually fix GLES2_UpdateTexture but rather off-loads the work onto GLES_UpdateTextureYUV by passing the appropriate pointer offsets into the contiguous block of YUV data. This is only done for SDL_PIXELFORMAT_YV12 and SDL_PIXELFORMAT_IYUV. The previous code is used for all other pixel formats.

This may not be optimal. I will leave that to someone who knows better to make that call.

This patch also contains the typo fix in GLES2_UpdateTextureYUV.

both looks the same patch!

I tried to mimic what was done for YUV in "opengl/SDL_render_opengl.c".

Maybe, we should also try to double-check that it works with a smaller RECT than the full image.

Thank you guys!
https://hg.libsdl.org/SDL/rev/b627c37a25e8

Created attachment 1710 [details]
patch to work correctly with RECT input

Here's a patch so that when a SDL_Rect is passed to SDL_UpdateTexture[YUV], it works better.

Created attachment 1711 [details]
patch to work correctly with RECT input (bis)

update the patch to work with RECT.

Applied, thanks!
https://hg.libsdl.org/SDL/rev/9dbfb553c555