From 44e8d579356eddab658fa9f3ca5f0504a0a924d7 Mon Sep 17 00:00:00 2001
From: Philip Rideout <philiprideout@gmail.com>
Date: Wed, 13 Nov 2019 12:31:46 -0800
Subject: [PATCH] Stream API: cleanup as per code review feedback.

---
 android/filament-android/src/main/cpp/Stream.cpp | 16 +++++++++-------
 .../java/com/google/android/filament/Stream.java |  4 ++--
 filament/backend/include/backend/DriverEnums.h   |  6 +++---
 filament/backend/src/opengl/PlatformEGL.cpp      | 15 +++++++++++----
 4 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/android/filament-android/src/main/cpp/Stream.cpp b/android/filament-android/src/main/cpp/Stream.cpp
index 271a926103..5391661890 100644
--- a/android/filament-android/src/main/cpp/Stream.cpp
+++ b/android/filament-android/src/main/cpp/Stream.cpp
@@ -37,11 +37,6 @@ typedef struct AHardwareBuffer AHardwareBuffer;
 
 #include <dlfcn.h>
 
-template <typename T>
-static void loadSymbol(T*& pfn, const char *symbol) noexcept {
-    pfn = (T*)dlsym(RTLD_DEFAULT, symbol);
-}
-
 #endif
 
 using namespace filament;
@@ -189,8 +184,15 @@ Java_com_google_android_filament_Stream_nSetAcquiredImage(JNIEnv* env, jclass, j
 
 #ifdef ANDROID
 
-    AHardwareBuffer* (*AHardwareBuffer_fromHardwareBuffer)(JNIEnv*, jobject);
-    loadSymbol(AHardwareBuffer_fromHardwareBuffer, "AHardwareBuffer_fromHardwareBuffer");
+    using PFN_FROMHARDWAREBUFFER = AHardwareBuffer* (*)(JNIEnv*, jobject);
+    static PFN_FROMHARDWAREBUFFER AHardwareBuffer_fromHardwareBuffer =
+            (PFN_FROMHARDWAREBUFFER) dlsym(RTLD_DEFAULT, "AHardwareBuffer_fromHardwareBuffer");
+
+    // This function is not available before NDK 15 or before Android 8.
+    if (!AHardwareBuffer_fromHardwareBuffer) {
+        __android_log_print(ANDROID_LOG_INFO, "Filament", "AHardwareBuffer_fromHardwareBuffer is not available.");
+        return;
+    }
 
     AHardwareBuffer* nativeBuffer = AHardwareBuffer_fromHardwareBuffer(env, hwbuffer);
     if (!nativeBuffer) {
diff --git a/android/filament-android/src/main/java/com/google/android/filament/Stream.java b/android/filament-android/src/main/java/com/google/android/filament/Stream.java
index e35dfcb315..4e44c3525c 100644
--- a/android/filament-android/src/main/java/com/google/android/filament/Stream.java
+++ b/android/filament-android/src/main/java/com/google/android/filament/Stream.java
@@ -40,10 +40,10 @@ public class Stream {
         /** Not synchronized but copy-free. Good for video. */
         NATIVE,
 
-        /** Synchronized, but GL-only and incurs copies. Good for AR on older devices. */
+        /** Synchronized, but GL-only and incurs copies. Good for AR on devices before API 26. */
         TEXTURE_ID,
 
-        /** Synchronized, copy-free, and take a release callback. Good for AR on newer devices. */
+        /** Synchronized, copy-free, and take a release callback. Good for AR but requires API 26+. */
         ACQUIRED,
     };
 
diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h
index 93ffab70b3..e625b392c0 100644
--- a/filament/backend/include/backend/DriverEnums.h
+++ b/filament/backend/include/backend/DriverEnums.h
@@ -626,9 +626,9 @@ enum class BlendFunction : uint8_t {
 
 //! Stream for external textures
 enum class StreamType {
-    NATIVE,
-    TEXTURE_ID,
-    ACQUIRED,
+    NATIVE,     //!< Not synchronized but copy-free. Good for video.
+    TEXTURE_ID, //!< Synchronized, but GL-only and incurs copies. Good for AR on devices before API 26.
+    ACQUIRED,   //!< Synchronized, copy-free, and take a release callback. Good for AR but requires API 26+.
 };
 
 //! Releases an ACQUIRED external texture, guaranteed to be called on the application thread.
diff --git a/filament/backend/src/opengl/PlatformEGL.cpp b/filament/backend/src/opengl/PlatformEGL.cpp
index 6a0127cc26..f870ee0543 100644
--- a/filament/backend/src/opengl/PlatformEGL.cpp
+++ b/filament/backend/src/opengl/PlatformEGL.cpp
@@ -552,22 +552,29 @@ backend::AcquiredImage PlatformEGL::createAcquiredImage(void* hwbuffer, backend:
     }
     // Note that this cannot be used to stream protected video (for now) because we do not set EGL_PROTECTED_CONTENT_EXT.
     EGLint attrs[] = { EGL_NONE, EGL_NONE };
-    EGLImageKHR eglImage = eglCreateImageKHR(eglGetCurrentDisplay(), EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, clientBuffer, attrs);
+    EGLImageKHR eglImage = eglCreateImageKHR(mEGLDisplay, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, clientBuffer, attrs);
     if (eglImage == EGL_NO_IMAGE_KHR) {
         slog.e << "eglCreateImageKHR returned no image." << io::endl;
         return {};
     }
 
     // Destroy the EGLImage before invoking the user's callback.
-    AcquiredImage* closure = new AcquiredImage();
+    struct Closure {
+        void* image;
+        backend::StreamCallback callback;
+        void* userData;
+        EGLDisplay display;
+    };
+    Closure* closure = new Closure();
     closure->callback = userCallback;
     closure->image = hwbuffer;
     closure->userData = userData;
+    closure->display = mEGLDisplay;
     auto patchedCallback = [](void* image, void* userdata) {
-        if (eglDestroyImageKHR(eglGetCurrentDisplay(), (EGLImageKHR) image) == EGL_FALSE) {
+        Closure* closure = (Closure*) userdata;
+        if (eglDestroyImageKHR(closure->display, (EGLImageKHR) image) == EGL_FALSE) {
             slog.e << "eglDestroyImageKHR failed." << io::endl;
         }
-        backend::AcquiredImage* closure = (backend::AcquiredImage*) userdata;
         closure->callback(closure->image, closure->userData);
         delete closure;
     };