[vlc-commits] [Git][videolan/vlc][master] 5 commits: qt: fix GetMediaSource returning null models in DeviceSourceProvider

[Steve Lhomme (@robUx4)]

Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
6a6da651 by Pierre Lamot at 2024-12-20T09:47:11+00:00
qt: fix GetMediaSource returning null models in DeviceSourceProvider

fix: #28907

- - - - -
5bc89f66 by Pierre Lamot at 2024-12-20T09:47:11+00:00
qt: don't append source name to model name if source is discarded in DeviceSourceProvider

- - - - -
3143296b by Pierre Lamot at 2024-12-20T09:47:11+00:00
qt: fix wrong callback called on MediaSourceModel::mediaRemoved

- - - - -
a5cca93e by Pierre Lamot at 2024-12-20T09:47:11+00:00
qt: fix unique connection can't be used with lambdas in NetworkDeviceModel

We need to use the lamdda to be able to retreive the shared pointer on the
caller, using the QObject::sender isn't enough to retrieve it. To ensure that
the connection is unique, we store the corresponding connection for the
(unlikely) case where the model would be re-initialized

- - - - -
f2379a0e by Pierre Lamot at 2024-12-20T09:47:11+00:00
qt: fix MediaSourceModel leak in DeviceSourceProvider

Storing a shared pointer in the connection may leak the object as
the connection is destroyed when the source is destroyed, but the
connection holds a reference. Instead, we store a weak pointer, the value will
remains valid as the connection has the same lifespan as the sender.

- - - - -


2 changed files:

- modules/gui/qt/network/devicesourceprovider.cpp
- modules/gui/qt/network/networkdevicemodel.cpp


Changes:

=====================================
modules/gui/qt/network/devicesourceprovider.cpp
=====================================
@@ -105,6 +105,8 @@ MediaSourceModel::~MediaSourceModel()
 
 void MediaSourceModel::init()
 {
+    assert(m_mediaSource);
+
     if (m_listenner)
         return;
 
@@ -196,6 +198,9 @@ SharedMediaSourceModel MediaSourceCache::getMediaSourceModel(vlc_media_source_pr
         vlc_media_source_provider_GetMediaSource(provider, name),
         false );
 
+    if (!mediaSource)
+        return nullptr;
+
     SharedMediaSourceModel item = SharedMediaSourceModel::create(mediaSource);
     m_cache[key] = item;
     return item;
@@ -254,15 +259,15 @@ void DeviceSourceProvider::init()
                 if ( nameFilter != '*' && nameFilter != sourceName )
                     continue;
 
-                ctx.name += ctx.name.isEmpty() ? qfu( meta->longname ) : ", " + qfu( meta->longname );
-
                 SharedMediaSourceModel mediaSource = MediaSourceCache::getInstance()->getMediaSourceModel(provider, meta->name);
-                //ensure this QObject don't live in the worker thread
-                mediaSource->moveToThread(thread);
 
                 if (!mediaSource)
                     continue;
 
+                //ensure this QObject don't live in the worker thread
+                mediaSource->moveToThread(thread);
+
+                ctx.name += ctx.name.isEmpty() ? qfu( meta->longname ) : ", " + qfu( meta->longname );
                 ctx.sources.push_back(mediaSource);
             }
         },


=====================================
modules/gui/qt/network/networkdevicemodel.cpp
=====================================
@@ -260,6 +260,10 @@ public:
             emit q->nameChanged();
         });
 
+        for (auto conn : m_sourceUpdateConnections)
+            QObject::disconnect(conn);
+        m_sourceUpdateConnections.clear();
+
         //itemsUpdated is called only once after init.
         QObject::connect(m_sourcesProvider.get(), &DeviceSourceProvider::itemsUpdated, q,
                 [this]()
@@ -270,16 +274,30 @@ public:
                 for (const SharedInputItem& media : source->getMedias())
                     onMediaAdded(source, media);
 
-                QObject::connect(
+                //don't store a strong reference in the lambda or this may leak the source
+                //as the connection is destroyed when the source is destroyed but the
+                //connection holds a reference
+                QWeakPointer<MediaSourceModel> weakSource{source};
+
+                auto conn = QObject::connect(
                     source.get(), &MediaSourceModel::mediaAdded,
-                    q_ptr, [this, source](SharedInputItem media) {
-                        onMediaAdded(source, media);
-                    }, Qt::UniqueConnection);
-                QObject::connect(
+                    q_ptr, [this, weakSource](SharedInputItem media) {
+                        //source is the signal emitter, it should always exist
+                        auto strongSource  = weakSource.toStrongRef();
+                        assert(strongSource);
+                        onMediaAdded(strongSource, media);
+                    });
+                m_sourceUpdateConnections.push_back(conn);
+
+                conn = QObject::connect(
                     source.get(), &MediaSourceModel::mediaRemoved,
-                    q_ptr, [this, source](SharedInputItem media) {
-                        onMediaAdded(source, media);
-                    }, Qt::UniqueConnection);
+                    q_ptr, [this, weakSource](SharedInputItem media) {
+                        auto strongSource  = weakSource.toStrongRef();
+                        assert(strongSource);
+                        onMediaRemoved(strongSource, media);
+                    });
+
+                m_sourceUpdateConnections.push_back(conn);
             }
 
             m_revision += 1;
@@ -374,6 +392,8 @@ public:
     NetworkDeviceModel::SDCatType m_sdSource = NetworkDeviceModel::CAT_UNDEFINED;
     QString m_sourceName; // '*' -> all sources
     QString m_name; // source long name
+
+    std::vector<QMetaObject::Connection> m_sourceUpdateConnections;
 };
 
 NetworkDeviceModel::NetworkDeviceModel( QObject* parent )



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/ba595bce6ce184ab2680dd185e5a6ccaf309ba2e...f2379a0e276597efdfe7a6aa970fb0f9ae21fbc3

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/ba595bce6ce184ab2680dd185e5a6ccaf309ba2e...f2379a0e276597efdfe7a6aa970fb0f9ae21fbc3
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance