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

[GStreamer (GNOME Bugzilla)]

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

Matthew Waters (ystreet00) <ystreet00 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ystreet00 at gmail.com

--- Comment #9 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #8)
> (In reply to Matthew Waters (ystreet00) from comment #6)
> > Review of attachment 346442 [details] [review] [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 :)

As long as this is always true, it should be fine.

> > ::: 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?

Yes, those are the two options :).  Pick one, I don't mind which.

> > ::: 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.

It seems that GLES2/gl2ext.h vs GLES3/*.h is another header minefield as things
that are in GLES2 aren't exposed in GLES3 headers. *sigh*.  I'm not sure what
to do here exactly but this is probably for another bug.

> > @@ +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?

Yes, exactly.

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