Re: [PATCH 1/2] wined3d: Add support for NV12 textures
-----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 :-(
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJS+MA3AAoJEN0/YqbEcdMw434P/36l/R7vl7k3mx24+cYw1mll DOqx4yov+V+I3ERFbQjWmzwvAKoxFbo2+Qn7AaEAegGk1x5G3cuw4jZmo+C4bUb0 uvePY9WbvZptGFdavLAEgYWaBtY6M9pNJkp1PuIehqeq9D3XP98AUH4awzWFbfcD /dQQGVow1D8hkp57iATzUpEwDZBGttAePxyYR/gj7r1FnqPpVD0NW/zWoLJBWMrq 4YQSZBmJQK3xq5mcVlajNUg0Ylh82fp0bzxCie8Zw1jNuahmbGEAaht87fzxWCMc DgdcCS19aZEa39kNT/0ft/vybIPNyHVA+Mon+JgYmEVQRxhJMCl6gYbEKYCWcs0f OZc6d5B8Z4PX9FiPTnofs49SRq2+Gh6YggJdjToY2YZ2iOGgD4A87EIj2iZQc+6w 9jz2SLIZPl4zrjBY5Ji31qyKy8c+emOO9KC1OdnkFPo6eUeIussjrCfX7r2mNyRs 2qNHLz2yrd8aG0+swYvHocZ/wnUeOtgaTYJef8BTEut0+iBsDJ/eQom1/84orhxJ oHmdTbfepXMEoQvhkMSu79gZBMnwvFERZngzOPjKuw0KS3XM+KIFOy1nFeVAMGvb AoVa39jkByIcF0tN6vuf12LdWtdMqkwKeNuF36iEwVAgzkr8pAU6xHL5QqY1QhuB zsCB0PSX3QSmNC/4s7k8 =r3du -----END PGP SIGNATURE-----
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
participants (2)
-
Martin Storsjö -
Stefan Dösinger