qt: Use a proxy object for access to the QML widget

QML can destroy the video widget at any time, leaving
us with a dangling pointer. Use a lock and a proxy
object to cope with that, and block in the widget
destructor if there are ongoing calls into the widget.

ext/qt/gstqtsink.cc

@@ -143,6 +143,7 @@ gst_qt_sink_class_init (GstQtSinkClass * klass)
 static void
 gst_qt_sink_init (GstQtSink * qt_sink)
 {
+  qt_sink->widget = QSharedPointer<QtGLVideoItemInterface>();
 }
 
 static void
@@ -152,9 +153,14 @@ gst_qt_sink_set_property (GObject * object, guint prop_id,
   GstQtSink *qt_sink = GST_QT_SINK (object);
 
   switch (prop_id) {
-    case PROP_WIDGET:
-      qt_sink->widget = static_cast<QtGLVideoItem *> (g_value_get_pointer (value));
+    case PROP_WIDGET: {
+      QtGLVideoItem *qt_item = static_cast<QtGLVideoItem *> (g_value_get_pointer (value));
+      if (qt_item)
+        qt_sink->widget = qt_item->getInterface();
+      else
+        qt_sink->widget.clear();
       break;
+    }
     case PROP_FORCE_ASPECT_RATIO:
       g_return_if_fail (qt_sink->widget);
       qt_sink->widget->setForceAspectRatio (g_value_get_boolean (value));
@@ -196,6 +202,8 @@ gst_qt_sink_finalize (GObject * object)
 
   _reset (qt_sink);
 
+  qt_sink->widget.clear();
+
   G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
@@ -207,7 +215,13 @@ gst_qt_sink_get_property (GObject * object, guint prop_id,
 
   switch (prop_id) {
     case PROP_WIDGET:
-      g_value_set_pointer (value, qt_sink->widget);
+      /* This is not really safe - the app needs to be
+       * sure the widget is going to be kept alive or
+       * this can crash */
+      if (qt_sink->widget)
+        g_value_set_pointer (value, qt_sink->widget->videoItem());
+      else
+        g_value_set_pointer (value, NULL);
       break;
     case PROP_FORCE_ASPECT_RATIO:
       if (qt_sink->widget)
@@ -287,16 +301,16 @@ gst_qt_sink_change_state (GstElement * element, GstStateChange transition)
         return GST_STATE_CHANGE_FAILURE;
       }
 
-      if (!qt_item_init_winsys (qt_sink->widget)) {
+      if (!qt_sink->widget->initWinSys()) {
         GST_ELEMENT_ERROR (element, RESOURCE, NOT_FOUND,
             ("%s", "Could not initialize window system"),
             (NULL));
         return GST_STATE_CHANGE_FAILURE;
       }
 
-      qt_sink->display = qt_item_get_display (qt_sink->widget);
-      qt_sink->context = qt_item_get_context (qt_sink->widget);
-      qt_sink->qt_context = qt_item_get_qt_context (qt_sink->widget);
+      qt_sink->display = qt_sink->widget->getDisplay();
+      qt_sink->context = qt_sink->widget->getContext();
+      qt_sink->qt_context = qt_sink->widget->getQtContext();
 
       if (!qt_sink->display || !qt_sink->context || !qt_sink->qt_context) {
         GST_ELEMENT_ERROR (element, RESOURCE, NOT_FOUND,
@@ -321,7 +335,8 @@ gst_qt_sink_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       break;
     case GST_STATE_CHANGE_PAUSED_TO_READY:
-      qt_item_set_buffer (qt_sink->widget, NULL);
+      if (qt_sink->widget)
+        qt_sink->widget->setBuffer(NULL);
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
       break;
@@ -365,10 +380,10 @@ gst_qt_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
   if (!gst_video_info_from_caps (&qt_sink->v_info, caps))
     return FALSE;
 
-  if (!qt_item_set_caps (qt_sink->widget, caps))
+  if (!qt_sink->widget)
     return FALSE;
 
-  return TRUE;
+  return qt_sink->widget->setCaps(caps);
 }
 
 static GstFlowReturn
@@ -380,7 +395,8 @@ gst_qt_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
 
   qt_sink = GST_QT_SINK (vsink);
 
-  qt_item_set_buffer (qt_sink->widget, buf);
+  if (qt_sink->widget)
+    qt_sink->widget->setBuffer(buf);
 
   return GST_FLOW_OK;
 }

ext/qt/gstqtsink.h

@@ -51,8 +51,6 @@ struct _GstQtSink
   /* <private> */
   GstVideoSink          parent;
 
-  QtGLVideoItem        *widget;
-
   GstVideoInfo          v_info;
   GstBufferPool        *pool;
 
@@ -60,7 +58,7 @@ struct _GstQtSink
   GstGLContext         *context;
   GstGLContext         *qt_context;
 
-  GstQtSinkPrivate  *priv;
+  QSharedPointer<QtGLVideoItemInterface> widget;
 };
 
 /**

ext/qt/qtitem.cc

@@ -30,6 +30,7 @@
 #include "gstqtglutility.h"
 
 #include <QtCore/QRunnable>
+#include <QtCore/QMutexLocker>
 #include <QtGui/QGuiApplication>
 #include <QtQuick/QQuickWindow>
 #include <QtQuick/QSGSimpleTextureNode>
@@ -123,11 +124,21 @@ QtGLVideoItem::QtGLVideoItem()
   connect(this, SIGNAL(windowChanged(QQuickWindow*)), this,
           SLOT(handleWindowChanged(QQuickWindow*)));
 
+  this->proxy = QSharedPointer<QtGLVideoItemInterface>(new QtGLVideoItemInterface(this));
+
   GST_DEBUG ("%p init Qt Video Item", this);
 }
 
 QtGLVideoItem::~QtGLVideoItem()
 {
+  /* Before destroying the priv info, make sure
+   * no qmlglsink's will call in again, and that
+   * any ongoing calls are done by invalidating the proxy
+   * pointer */
+  GST_INFO ("Destroying QtGLVideoItem and invalidating the proxy");
+  proxy->invalidateRef();
+  proxy.clear();
+
   g_mutex_clear (&this->priv->lock);
   if (this->priv->context)
     gst_object_unref(this->priv->context);
@@ -237,18 +248,25 @@ _reset (QtGLVideoItem * qt_item)
 }
 
 void
-qt_item_set_buffer (QtGLVideoItem * widget, GstBuffer * buffer)
+QtGLVideoItemInterface::setBuffer (GstBuffer * buffer)
 {
-  g_return_if_fail (widget != NULL);
-  g_return_if_fail (widget->priv->negotiated);
+  QMutexLocker locker(&lock);
 
-  g_mutex_lock (&widget->priv->lock);
+  if (qt_item == NULL)
+    return;
 
-  gst_buffer_replace (&widget->priv->buffer, buffer);
+  if (!qt_item->priv->negotiated) {
+    GST_WARNING ("Got buffer on unnegotiated QtGLVideoItem. Dropping");
+    return;
+  }
 
-  QMetaObject::invokeMethod(widget, "update", Qt::QueuedConnection);
+  g_mutex_lock (&qt_item->priv->lock);
 
-  g_mutex_unlock (&widget->priv->lock);
+  gst_buffer_replace (&qt_item->priv->buffer, buffer);
+
+  QMetaObject::invokeMethod(qt_item, "update", Qt::QueuedConnection);
+
+  g_mutex_unlock (&qt_item->priv->lock);
 }
 
 void
@@ -280,50 +298,53 @@ QtGLVideoItem::onSceneGraphInvalidated ()
 }
 
 gboolean
-qt_item_init_winsys (QtGLVideoItem * widget)
+QtGLVideoItemInterface::initWinSys ()
 {
+  QMutexLocker locker(&lock);
+
   GError *error = NULL;
 
-  g_return_val_if_fail (widget != NULL, FALSE);
+  if (qt_item == NULL)
+    return FALSE;
 
-  g_mutex_lock (&widget->priv->lock);
+  g_mutex_lock (&qt_item->priv->lock);
 
-  if (widget->priv->display && widget->priv->qt_context
-      && widget->priv->other_context && widget->priv->context) {
+  if (qt_item->priv->display && qt_item->priv->qt_context
+      && qt_item->priv->other_context && qt_item->priv->context) {
     /* already have the necessary state */
-    g_mutex_unlock (&widget->priv->lock);
+    g_mutex_unlock (&qt_item->priv->lock);
     return TRUE;
   }
 
-  if (!GST_IS_GL_DISPLAY (widget->priv->display)) {
+  if (!GST_IS_GL_DISPLAY (qt_item->priv->display)) {
     GST_ERROR ("%p failed to retrieve display connection %" GST_PTR_FORMAT,
-        widget, widget->priv->display);
-    g_mutex_unlock (&widget->priv->lock);
+        qt_item, qt_item->priv->display);
+    g_mutex_unlock (&qt_item->priv->lock);
     return FALSE;
   }
 
-  if (!GST_IS_GL_CONTEXT (widget->priv->other_context)) {
-    GST_ERROR ("%p failed to retrieve wrapped context %" GST_PTR_FORMAT, widget,
-        widget->priv->other_context);
-    g_mutex_unlock (&widget->priv->lock);
+  if (!GST_IS_GL_CONTEXT (qt_item->priv->other_context)) {
+    GST_ERROR ("%p failed to retrieve wrapped context %" GST_PTR_FORMAT, qt_item,
+        qt_item->priv->other_context);
+    g_mutex_unlock (&qt_item->priv->lock);
     return FALSE;
   }
 
-  widget->priv->context = gst_gl_context_new (widget->priv->display);
+  qt_item->priv->context = gst_gl_context_new (qt_item->priv->display);
 
-  if (!widget->priv->context) {
-    g_mutex_unlock (&widget->priv->lock);
+  if (!qt_item->priv->context) {
+    g_mutex_unlock (&qt_item->priv->lock);
     return FALSE;
   }
 
-  if (!gst_gl_context_create (widget->priv->context, widget->priv->other_context,
+  if (!gst_gl_context_create (qt_item->priv->context, qt_item->priv->other_context,
         &error)) {
     GST_ERROR ("%s", error->message);
-    g_mutex_unlock (&widget->priv->lock);
+    g_mutex_unlock (&qt_item->priv->lock);
     return FALSE;
   }
 
-  g_mutex_unlock (&widget->priv->lock);
+  g_mutex_unlock (&qt_item->priv->lock);
   return TRUE;
 }
 
@@ -403,68 +424,115 @@ _calculate_par (QtGLVideoItem * widget, GstVideoInfo * info)
 }
 
 gboolean
-qt_item_set_caps (QtGLVideoItem * widget, GstCaps * caps)
+QtGLVideoItemInterface::setCaps (GstCaps * caps)
 {
+  QMutexLocker locker(&lock);
   GstVideoInfo v_info;
 
-  g_return_val_if_fail (widget != NULL, FALSE);
   g_return_val_if_fail (GST_IS_CAPS (caps), FALSE);
   g_return_val_if_fail (gst_caps_is_fixed (caps), FALSE);
 
-  if (widget->priv->caps && gst_caps_is_equal_fixed (widget->priv->caps, caps))
+  if (qt_item == NULL)
+    return FALSE;
+
+  if (qt_item->priv->caps && gst_caps_is_equal_fixed (qt_item->priv->caps, caps))
     return TRUE;
 
   if (!gst_video_info_from_caps (&v_info, caps))
     return FALSE;
 
-  g_mutex_lock (&widget->priv->lock);
+  g_mutex_lock (&qt_item->priv->lock);
 
-  _reset (widget);
+  _reset (qt_item);
 
-  gst_caps_replace (&widget->priv->caps, caps);
+  gst_caps_replace (&qt_item->priv->caps, caps);
 
-  if (!_calculate_par (widget, &v_info)) {
-    g_mutex_unlock (&widget->priv->lock);
+  if (!_calculate_par (qt_item, &v_info)) {
+    g_mutex_unlock (&qt_item->priv->lock);
     return FALSE;
   }
 
-  widget->priv->v_info = v_info;
-  widget->priv->negotiated = TRUE;
+  qt_item->priv->v_info = v_info;
+  qt_item->priv->negotiated = TRUE;
 
-  g_mutex_unlock (&widget->priv->lock);
+  g_mutex_unlock (&qt_item->priv->lock);
 
   return TRUE;
 }
 
 GstGLContext *
-qt_item_get_qt_context (QtGLVideoItem * qt_item)
+QtGLVideoItemInterface::getQtContext ()
 {
-  g_return_val_if_fail (qt_item != NULL, NULL);
+  QMutexLocker locker(&lock);
 
-  if (!qt_item->priv->other_context)
+  if (!qt_item || !qt_item->priv->other_context)
     return NULL;
 
   return (GstGLContext *) gst_object_ref (qt_item->priv->other_context);
 }
 
 GstGLContext *
-qt_item_get_context (QtGLVideoItem * qt_item)
+QtGLVideoItemInterface::getContext ()
 {
-  g_return_val_if_fail (qt_item != NULL, NULL);
+  QMutexLocker locker(&lock);
 
-  if (!qt_item->priv->context)
+  if (!qt_item || !qt_item->priv->context)
     return NULL;
 
   return (GstGLContext *) gst_object_ref (qt_item->priv->context);
 }
 
 GstGLDisplay *
-qt_item_get_display (QtGLVideoItem * qt_item)
+QtGLVideoItemInterface::getDisplay() 
 {
-  g_return_val_if_fail (qt_item != NULL, NULL);
+  QMutexLocker locker(&lock);
 
-  if (!qt_item->priv->display)
+  if (!qt_item || !qt_item->priv->display)
     return NULL;
 
   return (GstGLDisplay *) gst_object_ref (qt_item->priv->display);
 }
+
+void
+QtGLVideoItemInterface::setDAR(gint num, gint den)
+{
+  QMutexLocker locker(&lock);
+  if (!qt_item)
+    return;
+  qt_item->setDAR(num, den);
+}
+
+void
+QtGLVideoItemInterface::getDAR(gint * num, gint * den)
+{
+  QMutexLocker locker(&lock);
+  if (!qt_item)
+    return;
+  qt_item->getDAR (num, den);
+}
+
+void
+QtGLVideoItemInterface::setForceAspectRatio(bool force_aspect_ratio)
+{
+  QMutexLocker locker(&lock);
+  if (!qt_item)
+    return;
+  qt_item->setForceAspectRatio(force_aspect_ratio);
+}
+
+bool
+QtGLVideoItemInterface::getForceAspectRatio()
+{
+  QMutexLocker locker(&lock);
+  if (!qt_item)
+    return FALSE;
+  return qt_item->getForceAspectRatio();
+}
+
+void
+QtGLVideoItemInterface::invalidateRef()
+{
+  QMutexLocker locker(&lock);
+  qt_item = NULL;
+}
+

ext/qt/qtitem.h

@@ -25,12 +25,40 @@
 #include <gst/gl/gl.h>
 
 #include "gstqtgl.h"
+#include <QtCore/QMutex>
 #include <QtQuick/QQuickItem>
 #include <QtGui/QOpenGLContext>
 #include <QtGui/QOpenGLFunctions>
 
 typedef struct _QtGLVideoItemPrivate QtGLVideoItemPrivate;
 
+class QtGLVideoItem;
+
+class QtGLVideoItemInterface : public QObject
+{
+    Q_OBJECT
+public:
+    QtGLVideoItemInterface (QtGLVideoItem *w) : qt_item (w), lock() {};
+
+    void invalidateRef();
+
+    void setBuffer (GstBuffer * buffer);
+    gboolean setCaps (GstCaps *caps);
+    gboolean initWinSys ();
+    GstGLContext *getQtContext();
+    GstGLContext *getContext();
+    GstGLDisplay *getDisplay();
+    QtGLVideoItem *videoItem () { return qt_item; };
+
+    void setDAR(gint, gint);
+    void getDAR(gint *, gint *);
+    void setForceAspectRatio(bool);
+    bool getForceAspectRatio();
+private:
+    QtGLVideoItem *qt_item;
+    QMutex lock;
+};
+
 class InitializeSceneGraph;
 
 class QtGLVideoItem : public QQuickItem, protected QOpenGLFunctions
@@ -45,6 +73,7 @@ public:
     void setForceAspectRatio(bool);
     bool getForceAspectRatio();
 
+    QSharedPointer<QtGLVideoItemInterface> getInterface() { return proxy; };
     /* private for C interface ... */
     QtGLVideoItemPrivate *priv;
 
@@ -57,22 +86,15 @@ protected:
     QSGNode * updatePaintNode (QSGNode * oldNode, UpdatePaintNodeData * updatePaintNodeData);
 
 private:
+
     friend class InitializeSceneGraph;
     void setViewportSize(const QSize &size);
     void shareContext();
 
     QSize m_viewportSize;
     bool m_openGlContextInitialized;
+
+    QSharedPointer<QtGLVideoItemInterface> proxy;
 };
 
-extern "C"
-{
-void            qt_item_set_buffer (QtGLVideoItem * widget, GstBuffer * buffer);
-gboolean        qt_item_set_caps (QtGLVideoItem * widget, GstCaps * caps);
-gboolean        qt_item_init_winsys (QtGLVideoItem * widget);
-GstGLContext *  qt_item_get_qt_context (QtGLVideoItem * qt_item);
-GstGLContext *  qt_item_get_context (QtGLVideoItem * qt_item);
-GstGLDisplay *  qt_item_get_display (QtGLVideoItem * qt_item);
-}
-
 #endif /* __QT_ITEM_H__ */