Android: Fix texture leak in QAndroidTextureVideoOutput

Simplify the ownership of the OpenGL resources and make it simpler
to release individual resources as needed. Previously we would
reset the texture handle without releasing it back to the system, making
us leak texture each time the video output was reset.

Also, be consistent with the type used for the texture handle.

Task-number: QTBUG-54340
Change-Id: Ic3b3c7322677a909e51aa5983fa99ccf8b290302
Reviewed-by: Yoann Lopes <yoann.lopes@qt.io>

4 files changed, 51 insertions, 55 deletions

diff --git a/src/plugins/android/src/common/qandroidvideooutput.cpp b/src/plugins/android/src/common/qandroidvideooutput.cpp
index f462a28d5..2010ec9e0 100644
--- a/src/plugins/android/src/common/qandroidvideooutput.cpp
+++ b/src/plugins/android/src/common/qandroidvideooutput.cpp
@@ -60,6 +60,22 @@ static const GLfloat g_texture_data[] = {
     0.f, 1.f
 };
 
+void OpenGLResourcesDeleter::deleteTextureHelper(quint32 id)
+{
+    if (id != 0)
+        glDeleteTextures(1, &id);
+}
+
+void OpenGLResourcesDeleter::deleteFboHelper(void *fbo)
+{
+    delete reinterpret_cast<QOpenGLFramebufferObject *>(fbo);
+}
+
+void OpenGLResourcesDeleter::deleteShaderProgramHelper(void *prog)
+{
+    delete reinterpret_cast<QOpenGLShaderProgram *>(prog);
+}
+
 
 class AndroidTextureVideoBuffer : public QAbstractVideoBuffer
 {
@@ -124,40 +140,6 @@ private:
     QImage m_image;
 };
 
-
-class OpenGLResourcesDeleter : public QObject
-{
-public:
-    OpenGLResourcesDeleter()
-        : m_textureID(0)
-        , m_fbo(0)
-        , m_program(0)
-    { }
-
-    ~OpenGLResourcesDeleter()
-    {
-        glDeleteTextures(1, &m_textureID);
-        delete m_fbo;
-        delete m_program;
-    }
-
-    void setTexture(quint32 id) {
-        if (m_textureID)
-            glDeleteTextures(1, &m_textureID);
-
-        m_textureID = id;
-    }
-
-    void setFbo(QOpenGLFramebufferObject *fbo) { m_fbo = fbo; }
-    void setShaderProgram(QOpenGLShaderProgram *prog) { m_program = prog; }
-
-private:
-    quint32 m_textureID;
-    QOpenGLFramebufferObject *m_fbo;
-    QOpenGLShaderProgram *m_program;
-};
-
-
 QAndroidTextureVideoOutput::QAndroidTextureVideoOutput(QObject *parent)
     : QAndroidVideoOutput(parent)
     , m_surface(0)
@@ -165,7 +147,6 @@ QAndroidTextureVideoOutput::QAndroidTextureVideoOutput(QObject *parent)
     , m_externalTex(0)
     , m_fbo(0)
     , m_program(0)
-    , m_glDeleter(0)
     , m_surfaceTextureCanAttachToContext(QtAndroidPrivate::androidSdkVersion() >= 16)
 {
 
@@ -175,8 +156,12 @@ QAndroidTextureVideoOutput::~QAndroidTextureVideoOutput()
 {
     clearSurfaceTexture();
 
-    if (m_glDeleter)
+    if (!m_glDeleter.isNull()) { // Make sure all of these are deleted on the render thread.
+        m_glDeleter->deleteFbo(m_fbo);
+        m_glDeleter->deleteShaderProgram(m_program);
+        m_glDeleter->deleteTexture(m_externalTex);
         m_glDeleter->deleteLater();
+    }
 }
 
 QAbstractVideoSurface *QAndroidTextureVideoOutput::surface() const
@@ -223,8 +208,7 @@ bool QAndroidTextureVideoOutput::initSurfaceTexture()
         // for the GL render thread to call us back to do it.
         if (QOpenGLContext::currentContext()) {
             glGenTextures(1, &m_externalTex);
-            m_glDeleter = new OpenGLResourcesDeleter;
-            m_glDeleter->setTexture(m_externalTex);
+            m_glDeleter.reset(new OpenGLResourcesDeleter);
         } else if (!m_externalTex) {
             return false;
         }
@@ -239,10 +223,9 @@ bool QAndroidTextureVideoOutput::initSurfaceTexture()
     } else {
         delete m_surfaceTexture;
         m_surfaceTexture = 0;
-        if (m_glDeleter)
-            m_glDeleter->deleteLater();
+        if (!m_glDeleter.isNull())
+            m_glDeleter->deleteTexture(m_externalTex);
         m_externalTex = 0;
-        m_glDeleter = 0;
     }
 
     return m_surfaceTexture != 0;
@@ -257,8 +240,14 @@ void QAndroidTextureVideoOutput::clearSurfaceTexture()
     }
 
     // Also reset the attached OpenGL texture
-    if (m_surfaceTextureCanAttachToContext)
+    // Note: The Android SurfaceTexture class does not release the texture on deletion,
+    // only if detachFromGLContext() called (API level >= 16), so we'll do it manually,
+    // on the render thread.
+    if (m_surfaceTextureCanAttachToContext) {
+        if (!m_glDeleter.isNull())
+            m_glDeleter->deleteTexture(m_externalTex);
         m_externalTex = 0;
+    }
 }
 
 AndroidSurfaceTexture *QAndroidTextureVideoOutput::surfaceTexture()
@@ -388,19 +377,17 @@ void QAndroidTextureVideoOutput::createGLResources()
     Q_ASSERT(QOpenGLContext::currentContext() != NULL);
 
     if (!m_glDeleter)
-        m_glDeleter = new OpenGLResourcesDeleter;
+        m_glDeleter.reset(new OpenGLResourcesDeleter);
 
     if (m_surfaceTextureCanAttachToContext && !m_externalTex) {
         m_surfaceTexture->detachFromGLContext();
         glGenTextures(1, &m_externalTex);
         m_surfaceTexture->attachToGLContext(m_externalTex);
-        m_glDeleter->setTexture(m_externalTex);
     }
 
     if (!m_fbo || m_fbo->size() != m_nativeSize) {
         delete m_fbo;
         m_fbo = new QOpenGLFramebufferObject(m_nativeSize);
-        m_glDeleter->setFbo(m_fbo);
     }
 
     if (!m_program) {
@@ -431,8 +418,6 @@ void QAndroidTextureVideoOutput::createGLResources()
         m_program->bindAttributeLocation("vertexCoordsArray", 0);
         m_program->bindAttributeLocation("textureCoordArray", 1);
         m_program->link();
-
-        m_glDeleter->setShaderProgram(m_program);
     }
 }
 
@@ -442,8 +427,7 @@ void QAndroidTextureVideoOutput::customEvent(QEvent *e)
         // This is running in the render thread (OpenGL enabled)
         if (!m_surfaceTextureCanAttachToContext && !m_externalTex) {
             glGenTextures(1, &m_externalTex);
-            m_glDeleter = new OpenGLResourcesDeleter; // will cleanup GL resources in the correct thread
-            m_glDeleter->setTexture(m_externalTex);
+            m_glDeleter.reset(new OpenGLResourcesDeleter); // We'll use this to cleanup GL resources in the correct thread
             emit readyChanged(true);
         }
     }
diff --git a/src/plugins/android/src/common/qandroidvideooutput.h b/src/plugins/android/src/common/qandroidvideooutput.h
index 0068a5809..09bd5d87e 100644
--- a/src/plugins/android/src/common/qandroidvideooutput.h
+++ b/src/plugins/android/src/common/qandroidvideooutput.h
@@ -44,7 +44,6 @@ class AndroidSurfaceTexture;
 class AndroidSurfaceHolder;
 class QOpenGLFramebufferObject;
 class QOpenGLShaderProgram;
-class OpenGLResourcesDeleter;
 class QAbstractVideoSurface;
 
 class QAndroidVideoOutput : public QObject
@@ -69,6 +68,19 @@ protected:
     QAndroidVideoOutput(QObject *parent) : QObject(parent) { }
 };
 
+class OpenGLResourcesDeleter : public QObject
+{
+    Q_OBJECT
+public:
+    void deleteTexture(quint32 id) { QMetaObject::invokeMethod(this, "deleteTextureHelper", Qt::AutoConnection, Q_ARG(quint32, id)); }
+    void deleteFbo(QOpenGLFramebufferObject *fbo) { QMetaObject::invokeMethod(this, "deleteFboHelper", Qt::AutoConnection, Q_ARG(void *, fbo)); }
+    void deleteShaderProgram(QOpenGLShaderProgram *prog) { QMetaObject::invokeMethod(this, "deleteShaderProgramHelper", Qt::AutoConnection, Q_ARG(void *, prog)); }
+
+private:
+    Q_INVOKABLE void deleteTextureHelper(quint32 id);
+    Q_INVOKABLE void deleteFboHelper(void *fbo);
+    Q_INVOKABLE void deleteShaderProgramHelper(void *prog);
+};
 
 class QAndroidTextureVideoOutput : public QAndroidVideoOutput
 {
@@ -108,7 +120,7 @@ private:
     quint32 m_externalTex;
     QOpenGLFramebufferObject *m_fbo;
     QOpenGLShaderProgram *m_program;
-    OpenGLResourcesDeleter *m_glDeleter;
+    QScopedPointer<OpenGLResourcesDeleter, QScopedPointerDeleteLater> m_glDeleter;
 
     bool m_surfaceTextureCanAttachToContext;
 
diff --git a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp
index bc7187e9c..cd6d41d84 100644
--- a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp
+++ b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp
@@ -56,7 +56,7 @@ static void notifyFrameAvailable(JNIEnv* , jobject, jlong id)
         Q_EMIT obj->frameAvailable();
 }
 
-AndroidSurfaceTexture::AndroidSurfaceTexture(unsigned int texName)
+AndroidSurfaceTexture::AndroidSurfaceTexture(quint32 texName)
     : QObject()
 {
     Q_STATIC_ASSERT(sizeof (jlong) >= sizeof (void *));
@@ -157,7 +157,7 @@ jobject AndroidSurfaceTexture::surfaceHolder()
     return m_surfaceHolder.object();
 }
 
-void AndroidSurfaceTexture::attachToGLContext(int texName)
+void AndroidSurfaceTexture::attachToGLContext(quint32 texName)
 {
     if (QtAndroidPrivate::androidSdkVersion() < 16 || !m_surfaceTexture.isValid())
         return;
diff --git a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h
index 3c41bf51d..1b82ad20e 100644
--- a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h
+++ b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h
@@ -45,7 +45,7 @@ class AndroidSurfaceTexture : public QObject
 {
     Q_OBJECT
 public:
-    explicit AndroidSurfaceTexture(unsigned int texName);
+    explicit AndroidSurfaceTexture(quint32 texName);
     ~AndroidSurfaceTexture();
 
     jobject surfaceTexture();
@@ -57,7 +57,7 @@ public:
     void release(); // API level 14
     void updateTexImage();
 
-    void attachToGLContext(int texName); // API level 16
+    void attachToGLContext(quint32 texName); // API level 16
     void detachFromGLContext(); // API level 16
 
     static bool initJNI(JNIEnv *env);