GNOME Bugzilla – Bug 743242
gl: texture targets in GstGLMemory
Last modified: 2016-02-04 06:57:53 UTC
So currently there's a problem of various zerocopy paths (avf, android amc, dmabuf, etc) requiring different texture targets to the nominal GL_TEXTURE_2D. Pretty much every texture target is used by at least one zerocopy path and so we require support in at least gstglupload/gstglcolorconvert to convert from these targets to GL_TEXTURE_2D (or allow passthrough so the element/application can handle it). For signalling this information between elements, I was thinking of three options. 1. add the required field to GstGLMemory and have everyone deal with it. (requires changing the world at once and is not backwards compatible at all) 2. add a new extra field in the memory:GstGLMemory caps say gl-texture-target (requires extra code to parse the extra field out of the caps that is not handled by gst_video_info_{to,from}_caps(). if the field doesn't exist, default to _2D) 3. A new meta solely to hold the texture target/s (simple, if it doesn't exist, default to _2D). (Open question should the target in the meta refer to entire buffer or be per memory according to gst_buffer_peek_memory (buf, i)) Ideally I would eventually like 1, then 3, then 2. Any other ideas?
I think 1) seems the cleanest approach, everything else looks more like trying to preserve the current API :) Let's break the API while it's still not too widely used.
We eventually need a combination of 1) and some way of signalling that this element/application only does RECTANGLE or EXTERNAL or 2D or some combination of the three which is what 2 and 3 are. Another option 4) is adding a caps feature for the different targets. The signalling part can happen later if we wire up glupload/gstglcolorconvert to convert to _2D from whatever input.
Lubosz implements 1) in bug 743345 (not sure how complete yet). The idea is that these texture target can be used directly in rendering, mixing, etc. So the best is if we can keep them around as long as possible before paying the cost of converting to TEXTURE_2D. Though, being able to do so in upload or convert is needed.
Agree, let's break the API. While we're at it, I'd like to make sure we get anything we need for stereo/multiview support in too.
(In reply to comment #4) > Agree, let's break the API. While we're at it, I'd like to make sure we get > anything we need for stereo/multiview support in too. I already made that step for 1.6 (libav/glimagesink direct rendering), so no need to worry.
Related to this are also custom shaders that might be necessary to handle these textures. I remember that for Android surfaces something was needed to crop the edges (but maybe that can be done with CROP meta instead?), but maybe more special shaders are needed for other things.
(In reply to comment #6) > Related to this are also custom shaders that might be necessary to handle these > textures. I remember that for Android surfaces something was needed to crop the > edges (but maybe that can be done with CROP meta instead?), but maybe more > special shaders are needed for other things. Indeed, Android video frames comes with an transformation matrix that need to be applied (they also do orientation with that).
(In reply to comment #2) > The signalling part can happen later if we wire up glupload/gstglcolorconvert > to convert to _2D from whatever input. I did this in https://github.com/alessandrod/gst-plugins-bad/commit/4184e2150a2de387b67ed71cdb3a6a969b663019 Using that branch I can now convert/upload/download GL_TEXTURE_RECTANGLE. As the commit message says I have left GL_TEXTURE_2D as the target used for internal textures used by glimagesink and friends since I think it's better to change that in a second step.
Interesting, though what would be the reason to convert to GL_TEXTURE_2D if we are to render directly on the screen ? Is there a way to avoid this GPU side conversion ?
(In reply to comment #9) > Interesting, though what would be the reason to convert to GL_TEXTURE_2D if we > are to render directly on the screen ? Is there a way to avoid this GPU side > conversion ? Yep and ultimately that's what we want to do. The change above is a step in that direction, since propagating non-2D textures all the way downstream requires knowing that they are not 2D in the first place.
I had a chat about this with Matthew and he pointed to cgit.collabora.com/git/user/lubosz/gst-plugins-bad.git/commit/?h=dmabuf-colorconvert&id=8d349a7a0a03492c5057b71cad9a01296244040c which implements the other half of the work (I did the work for wrapped textures, lubosz did it for new textures). Since I can only test wrapped textures and lubosz' work and mine are largely complementary, I'd like to push my change (assuming it gets reviewed :P) at some point this week so that I can push my vtdec changes that depend on it and have zerocopy working on OSX/iOS.
(In reply to comment #11) > Since I can only test wrapped textures and lubosz' work and mine are largely > complementary, I'd like to push my change (assuming it gets reviewed :P) at > some point this week so that I can push my vtdec changes that depend on it and > have zerocopy working on OSX/iOS. Let's not use the term zero copy here, since it's not, Daniel S. can explain ;-P. Note this work from Lubosz is explained at bug 743345 which has this bug as a blocker. Basically on most platform, the EGLImage created from DMABUF can only be binded to TEXTURE_EXTERNAL_EOS, which creates the same issue has having RECTANGLE from vtdec or EXTERNAL_EOS from AMC.
(I mean TEXTURE_OES but my finger won't cooperate)
(In reply to comment #12) > (In reply to comment #11) > > Since I can only test wrapped textures and lubosz' work and mine are largely > > complementary, I'd like to push my change (assuming it gets reviewed :P) at > > some point this week so that I can push my vtdec changes that depend on it and > > have zerocopy working on OSX/iOS. > > Let's not use the term zero copy here, since it's not, Daniel S. can explain > ;-P. Sorry we in the marketing department have decided to go for zerocopy
As a quick note, for now we only have 2 concrete cases (that have been implemented and tested, upstream or not): * gstglmemory or gstgleglimage_with_context -> GL_TEXTURE_2D * gstgleglimage_no_context -> GL_TEXTURE_EXTERNAL_OES
(In reply to comment #15) > As a quick note, for now we only have 2 concrete cases (that have been > implemented and tested, upstream or not): > * gstglmemory or gstgleglimage_with_context -> GL_TEXTURE_2D > * gstgleglimage_no_context -> GL_TEXTURE_EXTERNAL_OES Not sure what you mean, the EGLImage code we have seem to always do TEXTURE_2D (something that depends on the EGLImage internal format being packed or not, and the set of extensions available), did I miss something ? Also, Alessandro implementation adds TEXTURE_RECTANGLE, which is what vtdec produces. This bug isn't about EGLImage either, since we also want to support for EXTERNAL_OES as this is what Android decoder produce (without the intermediate EGLImage).
Please do cross references of the commits that are related to this bug here when merging please. And also tag these commit with the bug link so the cross reference is complete. == Commit: c7284a6390339f6ec39a59b43ad709678b5c1e1d URL: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=c7284a6390339f6ec39a59b43ad709678b5c1e1d Author: Alessandro Decina <alessandro.d@gmail.com> Date: Thu Jan 22 13:11:46 2015 +1100 gstglcolorconvert: relax caps features check Require caps to have the GST_CAPS_FEATURE_MEMORY_GL_MEMORY feature but allow them to have more features. == Commit: 3655e8b8bcfa5079dc26788596cb26b0c72a8900 URL: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=3655e8b8bcfa5079dc26788596cb26b0c72a8900 Author: Alessandro Decina <alessandro.d@gmail.com> Date: Wed Jan 28 00:48:27 2015 +1100 libgstgl: run a custom shader to convert YUV to RGB on mac and ios When GL_APPLE_ycbcr_422 is available, run a custom shader to convert GL_TEXTURE_RECTANGLE textures from YUV to RGB. See https://www.opengl.org/registry/specs/APPLE/ycbcr_422.txt Commit: 5f547c56001c25fcb352ed735ca79c0ff344322c URL: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=5f547c56001c25fcb352ed735ca79c0ff344322c Author: Alessandro Decina <alessandro.d@gmail.com> Date: Wed Jan 28 00:13:46 2015 +1100 gl: initial support for texture targets other than GL_TEXTURE_2D Make GstGLMemory hold the texture target (tex_target) the texture it represents (tex_id) is bound to. Modify gst_gl_memory_wrapped_texture and gst_gl_download_perform_with_data to take the texture target as an argument. This change is needed to support wrapping textures created outside libgstgl, which might be bound to a target other than GL_TEXTURE_2D. For example on OSX textures coming from VideoToolbox have target GL_TEXTURE_RECTANGLE. With this change we still keep (and sometimes imply) GL_TEXTURE_2D as the target of textures created with libgstgl. API: modify GstGLMemory API: modify gst_gl_memory_wrapped_texture API: gst_gl_download_perform_with_data
Regarding the custom shader commit see also my comment on the mailing list: I don't think this is the correct way of solving this problem. First of all see my follow-up commit c2492b3a6b7b06906272fcd78ab29400f842fca5 But also I think such custom shaders should be somehow attached to the GstGLMemory instead of magic hardcoding somewhere. See my comment in the related bug report too :) This needs some more work IMHO
And you can add notes to git commits with "git notes", to add the URL to the bug report.
(In reply to comment #18) > Regarding the custom shader commit see also my comment on the mailing list: > > I don't think this is the correct way of solving this problem. First of > all see my follow-up commit c2492b3a6b7b06906272fcd78ab29400f842fca5 > > But also I think such custom shaders should be somehow attached to the > GstGLMemory instead of magic hardcoding somewhere. See my comment in the > related bug report too :) > > This needs some more work IMHO As per IRC discussion I'm happy to iterate more on this. As I said tho so far I haven't seen evidence that would justify coming up with anything much more elaborate. Do we have other cases in mind/examples of custom shaders?
I'd say +1 on the field in the GstGlMemory (it *is* a property of those texture/surface after all, should therefore be present there, and as mentioned in #743345 it can potentially change per buffer) Thinking outloud here, if we need custom shaders ... can't those be put on in the uploadmeta ? Or can't we have both GstGlMemory + GstGlUploadMeta on a buffer at the same time ? In regards to AMC we need to potentially do some positioning/scaling/croping transformation on the outputted buffer, couldn't that fit in those custom shaders (instead of yet-another-custom-meta) ? I'll need to implement the always-use-OES-all-the-way next week for efficient/proper amc-gl zerocopy (instead of the current hackish convert-OES-to-2D-immediatly hack). I'll see where it goes.
What's left here ? I rebased this patch: https://github.com/CapOM/gst-plugins-bad/commit/60429b89e1b9a630786cc1af74a364bced6db5ba Maybe for now we could add a tex_target param to "gst_gl_shader_compile_with_default_vf_and_check" ? To at least have all simple default shaders in the same place: gstglshader.c
Isn't this fixed here now since other texture targets are supported? What's left?
(Also, EXTERNAL was not needed in the end of dmabuf)
commit e61d504556870adf2b5d58b86b09a3327816dec2 Author: Matthew Waters <matthew@centricular.com> Date: Thu Oct 29 00:44:26 2015 +1100 glmemory: add support for rectangle textures Add the various tokens/strings for the differnet texture types (2D, rect, oes) Changes the GLmemory api to include the GstGLTextureTarget in all relevant functions. Update the relevant caps/templates for 2D only textures.