GNOME Bugzilla – Bug 760363
Add support for creating CoglTexture from DMABUF
Last modified: 2021-06-10 11:20:07 UTC
This would help for ClutterGst where video decoders could hand us dmabufs directly instead of them doing the GL upload.
Created attachment 318579 [details] [review] Bump version post-branch
Created attachment 318580 [details] [review] CoglTexture: add support for creating texture from dmabufs On GLES1.1/2.0 we can create EGLImages from dmabufs from which we can create GL textures. This adds a new CoglTextureDmabufEGL class to add this support.
Created attachment 318583 [details] [review] Bump version post-branch
Review of attachment 318580 [details] [review]: ::: cogl/winsys/cogl-texture-dmabuf-egl.c @@ +58,3 @@ + +#ifndef GL_TEXTURE_EXTERNAL_OES +#define GL_TEXTURE_EXTERNAL_OES 0x8D65 I believe that the most portable way to implement this would be to using normal GL_TEXTURE_2D. That side effect is that no color conversion can take place, but no mesa driver implement conversion this way anyway (exception to swizzling). In GStreamer glupload element, we have used this method, so we require fewer formats from the driver to make it useful, and not having to use external_eos makes everything simplier. That being said, I believe this API you give the choice for which type of texture we are going to bind to the EGLImage. @@ +120,3 @@ + // the wrong format? + case COGL_PIXEL_FORMAT_RGBA_8888: + return DRM_FORMAT_BGRA8888; DRM format are expressed as the order you get when you load 4 bytes into a int register (using one load), While GStreamer and COGL seems to express is as the order found as if you index each bytes. This conversion is endianess specific, hence I believe the entire conversion function is not correct. Note that for libMALI it's simply implemented wrong, it uses DRM_FORMAT_BGRA8888 to expressed DRM_FORMAT_RGBA8888. They simply miss-understood. On Google recent ChromeBook this issue seems fixed in their own copy of the libMALI, so I beleive it will be fixed in the future. @@ +133,3 @@ + uint32_t width, + uint32_t height, + uint32_t offset, Should be offsets, strides and fds array to match the EGLImage from dmabuf attributes. You can have 1 FD for N planes (hence N offsets), or N FD for N planes (in which case all offsets are often all 0). @@ +148,3 @@ + + if (!_cogl_has_private_feature (ctx, + COGL_PRIVATE_FEATURE_TEXTURE_2D_FROM_EGL_IMAGE_EXTERNAL)) That's a bit weird to check this feature as right not you don't bind a TEXTURE_2D. Or maybe the name of that cogl feature is misleading ?
Review of attachment 318580 [details] [review]: ::: cogl/winsys/cogl-texture-dmabuf-egl.c @@ +58,3 @@ + +#ifndef GL_TEXTURE_EXTERNAL_OES +#define GL_TEXTURE_EXTERNAL_OES 0x8D65 Thanks, I just got this working using just TEXTURE_2D. Indeed the endianess of the pixel format (which you mention later) got me... @@ +120,3 @@ + // the wrong format? + case COGL_PIXEL_FORMAT_RGBA_8888: + return DRM_FORMAT_BGRA8888; Got it. I'm still a bit ensure about the right order for RG88 though :/ @@ +133,3 @@ + uint32_t width, + uint32_t height, + uint32_t offset, Similarly to what you said about how the gstreamer gl plugins implemented this, I thought it would be simpler to create one texture per plane and let the user of this api write the shader to combine everything properly. That sounds simpler to me for now and we can always add another _new_whatever() method to deal with M planes & N fds and all the complicated logic to get the combination right later. @@ +148,3 @@ + + if (!_cogl_has_private_feature (ctx, + COGL_PRIVATE_FEATURE_TEXTURE_2D_FROM_EGL_IMAGE_EXTERNAL)) That's the GL_OES_EGL_image_external check. But that's going away.
Created attachment 318829 [details] [review] CoglTexture: add support for creating texture from dmabufs On GLES1.1/2.0 we can create EGLImages from dmabufs from which we can create GL textures. This adds a new CoglTextureDmabufEGL class to add this support.
(In reply to Lionel Landwerlin from comment #6) > Created attachment 318829 [details] [review] [review] > CoglTexture: add support for creating texture from dmabufs > > On GLES1.1/2.0 we can create EGLImages from dmabufs from which we can create > GL textures. > > This adds a new CoglTextureDmabufEGL class to add this support. Cogl texture types are currently more comparable to GL texture targets, so I wouldn't have expected a new texture object type to support dmabuf if the resulting texture behaves the same as a normal TEXTURE_2D texture. I'd imagine only adding a new constructor something like cogl_texture_2d_new_from_dmabuf. E.g. see _cogl_egl_texture_2d_new_from_image which I guess is similar in nature to this. This dmabuf based constructor might even be doable as a small shim that calls _cogl_egl_texture_2d_new_from_image internally.
Created attachment 318922 [details] [review] CoglTexture: add support for creating texture from dmabufs On GLES1.1/2.0 we can create EGLImages from dmabufs from which we can create GL textures. This adds a new CoglTextureDmabufEGL class to add this support.
Created attachment 318923 [details] [review] CoglTexture: add support for creating 2d texture from dmabufs On GLES1.1/2.0 we can create EGLImages from dmabufs from which we can create GL textures. This adds a new cogl_texture_2d_new_from_dmabuf method to support this feature.
Review of attachment 318922 [details] [review]: Moving to 2d textures.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of cogl, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a ticket at https://gitlab.gnome.org/GNOME/cogl/-/issues/ Thank you for your understanding and your help.