GStreamer: Only register one typefind function

Each time a GStreamer session was created it would register the same
callback function, but each time with a new user data, which in our
case was a pointer to the session. The pointer would then be used
without verification of its validity. The problem with this is that
the typefind function is static and used for all sessions, making
multiple sessions impossible.
The documentation for gst_type_find_register() also clearly specifies
that the supplied user data needs to valid as long as the plugin is
loaded, which of course was not the case.

With this change only one callback function will be registered, and
each session will register itself with a typefind delegate, that will
keep track of sessions that are still alive.

Note: This only makes sure that the typefind function is called on
a valid session, it does not make sure that the type actually
belong to the session, which is a separate issue.

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

2 files changed, 56 insertions, 12 deletions

diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp
index 67bfa628f..76a56aca7 100644
--- a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp
+++ b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp
@@ -61,12 +61,59 @@
 #include <QtCore/qdebug.h>
 #include <QtCore/qdir.h>
 #include <QtCore/qstandardpaths.h>
+#include <QtCore/qvector.h>
+#include <QtCore/qmutex.h>
 
 //#define DEBUG_PLAYBIN
 //#define DEBUG_VO_BIN_DUMP
 
 QT_BEGIN_NAMESPACE
 
+class TypefindDelegator
+{
+public:
+    TypefindDelegator()
+    {
+        Q_ASSERT(gst_type_find_register(0, "playlist", GST_RANK_MARGINAL, notifySessions, 0, 0, 0, 0) == TRUE);
+    }
+
+    void add(QGstreamerPlayerSession *session)
+    {
+        QMutexLocker locker(&m_mtx);
+        m_sessions.append(session);
+    }
+
+    void remove(QGstreamerPlayerSession *session)
+    {
+        QMutexLocker locker(&m_mtx);
+        const int idx = m_sessions.indexOf(session);
+        if (idx != -1)
+            m_sessions.remove(idx);
+    }
+
+private:
+    static void notifySessions(GstTypeFind *find, gpointer /* unused */)
+    {
+        QMutexLocker locker(&m_mtx);
+        SessionList::const_iterator it = m_sessions.constBegin();
+        SessionList::const_iterator end = m_sessions.constEnd();
+
+        while (it != end) {
+            (*it)->playlistTypeFindFunction(find);
+            ++it;
+        }
+    }
+
+    typedef QVector<QGstreamerPlayerSession *> SessionList;
+    static SessionList m_sessions;
+    static QMutex m_mtx;
+};
+
+TypefindDelegator::SessionList TypefindDelegator::m_sessions;
+QMutex TypefindDelegator::m_mtx;
+
+Q_GLOBAL_STATIC(TypefindDelegator, g_typeRegister);
+
 static bool usePlaybinVolume()
 {
     static enum { Yes, No, Unknown } status = Unknown;
@@ -147,10 +194,7 @@ QGstreamerPlayerSession::QGstreamerPlayerSession(QObject *parent)
      m_isLiveSource(false),
      m_isPlaylist(false)
 {
-    gboolean result = gst_type_find_register(0, "playlist", GST_RANK_MARGINAL, playlistTypeFindFunction, 0, 0, this, 0);
-    Q_ASSERT(result == TRUE);
-    Q_UNUSED(result);
-
+    g_typeRegister->add(this);
     m_playbin = gst_element_factory_make(QT_GSTREAMER_PLAYBIN_ELEMENT_NAME, NULL);
     if (m_playbin) {
         //GST_PLAY_FLAG_NATIVE_VIDEO omits configuration of ffmpegcolorspace and videoscale,
@@ -251,6 +295,8 @@ QGstreamerPlayerSession::QGstreamerPlayerSession(QObject *parent)
 
 QGstreamerPlayerSession::~QGstreamerPlayerSession()
 {
+    g_typeRegister->remove(this);
+
     if (m_playbin) {
         stop();
 
@@ -1890,15 +1936,13 @@ void QGstreamerPlayerSession::resumeVideoProbes()
         m_videoProbe->stopFlushing();
 }
 
-void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpointer userData)
+void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find)
 {
-    QGstreamerPlayerSession* session = (QGstreamerPlayerSession*)userData;
-
     gchar *strval = 0;
 #if GST_CHECK_VERSION(1,0,0)
-    g_object_get(G_OBJECT(session->m_playbin), "current-uri", &strval, NULL);
+    g_object_get(G_OBJECT(m_playbin), "current-uri", &strval, NULL);
 #else
-    g_object_get(G_OBJECT(session->m_playbin), "uri", &strval, NULL);
+    g_object_get(G_OBJECT(m_playbin), "uri", &strval, NULL);
 #endif
     const QString uri(QString::fromUtf8(strval));
     g_free(strval);
@@ -1912,7 +1956,7 @@ void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpoint
     while (length > 0) {
         const guint8 *data = gst_type_find_peek(find, 0, length);
         if (data) {
-            session->m_isPlaylist = (QPlaylistFileParser::findPlaylistType(uri, 0, data, length) != QPlaylistFileParser::UNKNOWN);
+            m_isPlaylist = (QPlaylistFileParser::findPlaylistType(uri, 0, data, length) != QPlaylistFileParser::UNKNOWN);
             return;
         }
         length >>= 1; // for HTTP files length is not available,
diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h
index e5d5498c7..fdfb416bd 100644
--- a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h
+++ b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h
@@ -131,6 +131,8 @@ public:
 
     void endOfMediaReset();
 
+    void playlistTypeFindFunction(GstTypeFind *find);
+
 public slots:
     void loadFromUri(const QNetworkRequest &url);
     void loadFromStream(const QNetworkRequest &url, QIODevice *stream);
@@ -192,8 +194,6 @@ private:
     void flushVideoProbes();
     void resumeVideoProbes();
 
-    static void playlistTypeFindFunction(GstTypeFind *find, gpointer userData);
-
     QNetworkRequest m_request;
     QMediaPlayer::State m_state;
     QMediaPlayer::State m_pendingState;