Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Padded, non-contiguous YUV does not display correctly using OpenGL ES 2.0 renderer #1526

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Padded, non-contiguous YUV does not display correctly using OpenGL ES 2.0 renderer #1526

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2014-06-20 15:09:14 +0000, Alvin wrote:

The new OpenGL ES 2.0 YUV Texture support does not correctly display padded, non-contiguous YUV data.

I am using SDL2 60edb019f0fe (as provided by 'hg id --id') from Mercurial.

The YUV data I am using is provided by the FFMPEG family of libraries. According to FFMPEG's documentation, "The linesize [pitch] may be larger than the size of usable data -- there may be extra padding present for performance reasons."

The dimensions of the video file that I am using are 480x360. What I get from FFMPEG is a Ypitch of 512, and Upitch and Vpitch are both 256.

When I pack new Y, U and V buffers with only the "usable" data (Ypitch is 480 and Upitch and Vpitch are both 240), and use those new buffers, the image is display correctly.

It appears that the Ypitch, Upitch and Vpitch parameters are not being used by SDL_UpdateYUVTexture().

I use SDL_PIXELFORMAT_YV12 for my YUV texture, however, the same results are seen when I use SDL_PIXELFORMAT_IYUV.

Not sure if this is related or not, but when I render the YUV texture (padded and unpadded) to a RGB24 texture, the resulting image is greyscale (or could by just the Y channel).

The URL field for this bug entry is set to my email (SDL mailing list archive) which includes an example image of what I see when rendering padded, non-contiguous YUV data.

On 2014-06-21 19:39:32 +0000, Sam Lantinga wrote:

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

On 2014-06-23 07:17:38 +0000, Sylvain wrote:

Seems broken, in GLES2_TexSubImage2D:

  • src = (Uint8 *)data;
  • src = (Uint8 *)pixels;

On 2014-06-23 12:26:27 +0000, Gabriel Jacobo wrote:

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

On 2014-06-23 14:12:52 +0000, Alvin wrote:

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

On 2014-06-23 14:15:43 +0000, Alvin wrote:

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

On 2014-06-23 14:38:06 +0000, Alvin wrote:

Created attachment 1704
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.

On 2014-06-23 17:58:49 +0000, Sylvain wrote:

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.

On 2014-06-24 06:22:34 +0000, Sylvain wrote:

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

On 2014-06-24 13:21:48 +0000, Alvin wrote:

Created attachment 1705
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!

On 2014-06-24 16:27:21 +0000, Sylvain wrote:

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.

On 2014-06-24 17:18:33 +0000, Alvin wrote:

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.

On 2014-06-24 19:07:44 +0000, Sylvain wrote:

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 WHbpp 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 ...

On 2014-06-24 19:26:50 +0000, Alvin wrote:

(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 WHbpp 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.

On 2014-06-24 20:19:13 +0000, Sylvain wrote:

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

Maybe as to be reviewed ...

On 2014-06-24 20:20:08 +0000, Sylvain wrote:

Created attachment 1707
patch for contiguous YUV

patch for contiguous YUV

On 2014-06-24 20:21:54 +0000, Sylvain wrote:

Comment on attachment 1707
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);
    }

On 2014-06-24 20:24:26 +0000, Sylvain wrote:

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

On 2014-06-24 20:39:02 +0000, Alvin wrote:

Created attachment 1708
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.

On 2014-06-25 06:51:19 +0000, Sylvain wrote:

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.

On 2014-06-25 07:59:12 +0000, Sam Lantinga wrote:

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

On 2014-06-25 17:21:58 +0000, Sylvain wrote:

Created attachment 1710
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.

On 2014-06-25 17:29:57 +0000, Sylvain wrote:

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

update the patch to work with RECT.

On 2014-06-26 04:07:19 +0000, Sam Lantinga wrote:

Applied, thanks!
https://hg.libsdl.org/SDL/rev/9dbfb553c555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@SDLBugzilla