[Bug 779067] glupload: Add support for Vivante DirectTexture uploads

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=779067

--- Comment #8 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Matthew Waters (ystreet00) from comment #6)
> Review of attachment 346442 [details] [review]:
> 
> An open question is how do we know the GstPhysMemory is usable by the
> Vivante extension?

If you have hardware with that, you'll have that function in your GL library.
And on that hardware the only kind of "physical memory" that will be available
is one that can be used here :)

> ::: configure.ac
> @@ +892,3 @@
>  
> +dnl check for Vivante DirectVIV support
> +AC_CHECK_LIB(GLESv2, glTexDirectVIV, [HAVE_VIV_DIRECTVIV=yes],
> [HAVE_VIV_DIRECTVIV=no])
> 
> I think this is the wrong thing to be checking here as this needs to be
> checked at runtime by the GL function loading infrastructure.
> 
> Do we even need to check for vivante specific things here?

If we check at runtime, then we would always compile it in although it's
useless almost everywhere else. 

How would we include it in the function loading infrastructure, other than
adding more functions to the GL prototypes? We could probably load it
dynamically when the uploader is created?

> ::: gst-libs/gst/gl/Makefile.am
> @@ +147,3 @@
>  	$(GST_ALL_LDFLAGS) \
> +	$(GST_LT_LDFLAGS) \
> +
> $(top_builddir)/gst-libs/gst/allocators/libgstbadallocators-
> @GST_API_VERSION at .la
> 
> Don't .la libraries go in _LIBADD?

Yes

> ::: gst-libs/gst/gl/gstglupload.c
> @@ +39,3 @@
> +#if GST_GL_HAVE_VIV_DIRECTVIV
> +#include <gst/allocators/gstphysmemory.h>
> +#include <GLES2/gl2ext.h>
> 
> This include should be in gstglapi.h

The problem is that gstglapi.h will include GLES3/*.h if available instead...
and those don't declare the functions for whatever reason although they are
available on work. Not a problem if we dynamically load them though.

> @@ +1395,3 @@
> +      gl_format, (void **) &unmap_data->map.data,
> +      &((GstPhysMemory *) in_mem)->phys_addr);
> +  glTexDirectInvalidateVIV (GL_TEXTURE_2D);
> 
> These should use the functions in GstGLFuncs in order to use the correct GL
> functions for the correct context/thread.
> 
> Which results in, even if GST_GL_HAVE_VIV_DIRECTVIV is true (which I don't
> think we need at all), glTexDirectVIVMap and glTexDirectInvalidateVIV may
> not exist at runtime and should be a condition for choosing to upload with
> this uploader.


So would you suggest to a) check at configure if the symbol is there to decide
whether to compile in that codepath or not, and b) load the symbol dynamically
when the uploader is created?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.