-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
The shader looks ok to me. Please add a test - see yuv_color_test in dlls/d3d9/tests/visual.c as an example, although it might not be feasible to integrate a NV12 test into the UYVY / YUY2 test loop.
Since the UYVY / YUY2 test already covers the color space conversion parts I think a NV12 test should make sure that the coordinate selection works properly. Test that e.g. a 4x4 or 8x8 texture produces the correct color pattern.
Am 2014-02-10 12:08, schrieb Martin Storsjo:
This fixes video playback with the sample app from OpenH264.
I haven't been able to test the GL_TEXTURE_RECTANGLE_ARB codepath here - if I explicitly disable gl_info->supported[ARB_TEXTURE_NON_POWER_OF_TWO] the same app crashes. Similarly for YV12 (which already exists and works), if I disable ARB_TEXTURE_NON_POWER_OF_TWO there, the app crashes and the video output is also garbled (the luma channel seems to be right but chroma is handled incorrectly).
I don't know what could be wrong with the texture rectangle codepath. I tested it when I wrote it for other formats, but it's quite possible that it has suffered from bitrot in the years since then. Also keep in mind that wined3d assumes limited ARB_TEXTURE_NON_POWER_OF_TWO support if OpenGL 2.0 is supported.
@@ -6772,6 +6772,7 @@ struct arbfp_blit_priv { GLenum yuy2_rect_shader, yuy2_2d_shader; GLenum uyvy_rect_shader, uyvy_2d_shader; GLenum yv12_rect_shader, yv12_2d_shader;
- GLenum nv12_rect_shader, nv12_2d_shader; GLenum p8_rect_shader, p8_2d_shader; GLuint palette_texture;
};
It might be a good idea to replace those 10 shaders and the poorly scaling search for them with a rbtree. Yeah, I know, I started this mess, but back then it was just 4 shaders and we've been too lazy to clean this up since then :-\ .
- switch(textype) {
case GL_TEXTURE_2D: tex = "2D"; break;case GL_TEXTURE_RECTANGLE_ARB: tex = "RECT"; break;default:FIXME("Implement nv12 correction for non-2d, non-rect textures\n");return FALSE;- }
Regarding the code style please use the style found in dlls/wined3d/cs.c. We're migrating the code style of wined3d to this style, but since style-only changes are frowned upon we're doing this as we go. I'm sorry for the mess :-(
On Mon, 10 Feb 2014, Stefan Dösinger wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
The shader looks ok to me. Please add a test - see yuv_color_test in dlls/d3d9/tests/visual.c as an example, although it might not be feasible to integrate a NV12 test into the UYVY / YUY2 test loop.
Since the UYVY / YUY2 test already covers the color space conversion parts I think a NV12 test should make sure that the coordinate selection works properly. Test that e.g. a 4x4 or 8x8 texture produces the correct color pattern.
Ok, I've made such a test, will resend the full patchset soon.
Am 2014-02-10 12:08, schrieb Martin Storsjo:
This fixes video playback with the sample app from OpenH264.
I haven't been able to test the GL_TEXTURE_RECTANGLE_ARB codepath here - if I explicitly disable gl_info->supported[ARB_TEXTURE_NON_POWER_OF_TWO] the same app crashes. Similarly for YV12 (which already exists and works), if I disable ARB_TEXTURE_NON_POWER_OF_TWO there, the app crashes and the video output is also garbled (the luma channel seems to be right but chroma is handled incorrectly).
I don't know what could be wrong with the texture rectangle codepath. I tested it when I wrote it for other formats, but it's quite possible that it has suffered from bitrot in the years since then. Also keep in mind that wined3d assumes limited ARB_TEXTURE_NON_POWER_OF_TWO support if OpenGL 2.0 is supported.
Yeah, I might have a look later if I can figure it out. It's probably not of highest priority at the moment anyway.
@@ -6772,6 +6772,7 @@ struct arbfp_blit_priv { GLenum yuy2_rect_shader, yuy2_2d_shader; GLenum uyvy_rect_shader, uyvy_2d_shader; GLenum yv12_rect_shader, yv12_2d_shader;
- GLenum nv12_rect_shader, nv12_2d_shader; GLenum p8_rect_shader, p8_2d_shader; GLuint palette_texture;
};
It might be a good idea to replace those 10 shaders and the poorly scaling search for them with a rbtree. Yeah, I know, I started this mess, but back then it was just 4 shaders and we've been too lazy to clean this up since then :-\ .
Ok, I made a patch that does this, on top of the NV12 patch. If requested I can reorder them but it's a bit of work.
- switch(textype) {
case GL_TEXTURE_2D: tex = "2D"; break;case GL_TEXTURE_RECTANGLE_ARB: tex = "RECT"; break;default:FIXME("Implement nv12 correction for non-2d, non-rect textures\n");return FALSE;- }
Regarding the code style please use the style found in dlls/wined3d/cs.c. We're migrating the code style of wined3d to this style, but since style-only changes are frowned upon we're doing this as we go. I'm sorry for the mess :-(
Ok, I've tried to make the newly added code match this as well as possible.
// Martin
-
Martin Storsjö -
Stefan Dösinger