[vlc-commits] [Git][videolan/vlc][master] qt: ensure network models aren't as cachable too early

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Tue Apr 30 06:13:44 UTC 2024



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
2212ef87 by Pierre Lamot at 2024-04-30T05:41:15+00:00
qt: ensure network models aren't as cachable too early

most network models are loaded asynchronously, if the cache is built right after
the model initialisation, data isn't loaded yet and the first model version is
built on an empty list, which can lead to a transitory state where the view
assumes it's not loading anymore and that no data is present

- - - - -


8 changed files:

- modules/gui/qt/Makefile.am
- modules/gui/qt/meson.build
- modules/gui/qt/network/networkdevicemodel.cpp
- modules/gui/qt/network/networkmediamodel.cpp
- modules/gui/qt/network/networksourcesmodel.cpp
- modules/gui/qt/network/servicesdiscoverymodel.cpp
- modules/gui/qt/network/standardpathmodel.cpp
- + modules/gui/qt/util/locallistbasemodel.hpp


Changes:

=====================================
modules/gui/qt/Makefile.am
=====================================
@@ -308,6 +308,7 @@ libqt_plugin_la_SOURCES = \
 	util/listcache.hxx util/listcache.hpp \
 	util/listcacheloader.hpp \
 	util/locallistcacheloader.hpp \
+	util/locallistbasemodel.hpp \
 	util/navigation_history.cpp util/navigation_history.hpp \
 	util/item_key_event_filter.cpp \
 	util/item_key_event_filter.hpp \


=====================================
modules/gui/qt/meson.build
=====================================
@@ -129,6 +129,7 @@ moc_headers = files(
     'util/keyhelper.hpp',
     'util/listcache.hpp',
     'util/locallistcacheloader.hpp',
+    'util/locallistbasemodel.hpp',
     'util/navigation_history.hpp',
     'util/item_key_event_filter.hpp',
     'util/effects_image_provider.hpp',
@@ -692,4 +693,3 @@ if qt6_dep.found()
         'dependencies': [qt6_dep, qt_extra_deps],
     }
 endif
-


=====================================
modules/gui/qt/network/networkdevicemodel.cpp
=====================================
@@ -28,8 +28,7 @@
 #include "playlist/playlist_controller.hpp"
 
 #include "util/shared_input_item.hpp"
-#include "util/base_model_p.hpp"
-#include "util/locallistcacheloader.hpp"
+#include "util/locallistbasemodel.hpp"
 
 namespace
 {
@@ -158,17 +157,16 @@ bool ListCache<NetworkDeviceItemPtr>::compareItems(const NetworkDeviceItemPtr& a
 // NetworkDeviceModelPrivate
 
 class NetworkDeviceModelPrivate
-    : public BaseModelPrivateT<NetworkDeviceItemPtr>
-    , public LocalListCacheLoader<NetworkDeviceItemPtr>::ModelSource
+    : public LocalListBaseModelPrivate<NetworkDeviceItemPtr>
 {
     Q_DECLARE_PUBLIC(NetworkDeviceModel)
 public:
     NetworkDeviceModelPrivate(NetworkDeviceModel * pub)
-        : BaseModelPrivateT<NetworkDeviceItemPtr>(pub)
+        : LocalListBaseModelPrivate<NetworkDeviceItemPtr>(pub)
         , m_items(0, NetworkDeviceItemHash{}, NetworkDeviceItemEqual{})
     {}
 
-    NetworkDeviceModelLoader::ItemCompare getSortFunction() const
+    NetworkDeviceModelLoader::ItemCompare getSortFunction() const override
     {
         if (m_sortCriteria == "mrl")
         {
@@ -186,13 +184,6 @@ public:
         }
     }
 
-    std::unique_ptr<ListCacheLoader<NetworkDeviceItemPtr>> createLoader() const override
-    {
-        return std::make_unique<NetworkDeviceModelLoader>(
-            this, m_searchPattern,
-            getSortFunction());
-    }
-
     bool initializeModel() override
     {
         Q_Q(NetworkDeviceModel);
@@ -240,6 +231,11 @@ public:
                 return false;
             m_listeners.push_back( std::move( l ) );
         }
+
+        //service discovery don't notify preparse end
+        m_loading = false;
+        emit q->loadingChanged();
+
         return m_listeners.empty() == false;
     }
 
@@ -293,7 +289,7 @@ public:
 
         if (dataChanged)
         {
-            m_modelRevision += 1;
+            m_revision += 1;
             invalidateCache();
         }
     }
@@ -336,17 +332,12 @@ public:
         }
         if (dataChanged)
         {
-            m_modelRevision += 1;
+            m_revision += 1;
             invalidateCache();
         }
     }
 
 public: //LocalListCacheLoader::ModelSource
-    size_t getModelRevision() const override
-    {
-        return m_modelRevision;
-    }
-
     std::vector<NetworkDeviceItemPtr> getModelData(const QString& pattern) const override
     {
         std::vector<NetworkDeviceItemPtr> items;
@@ -369,7 +360,6 @@ public: //LocalListCacheLoader::ModelSource
     }
 
 public:
-    size_t m_modelRevision = 0;
     NetworkDeviceItemSet m_items;
     std::vector<std::unique_ptr<MediaTreeListener>> m_listeners;
 };


=====================================
modules/gui/qt/network/networkmediamodel.cpp
=====================================
@@ -21,8 +21,7 @@
 
 #include "maininterface/mainctx.hpp"
 
-#include "util/base_model_p.hpp"
-#include "util/locallistcacheloader.hpp"
+#include "util/locallistbasemodel.hpp"
 
 #include "playlist/media.hpp"
 #include "playlist/playlist_controller.hpp"
@@ -75,14 +74,13 @@ bool ListCache<NetworkMediaItemPtr>::compareItems(const NetworkMediaItemPtr& a,
 // NetworkMediaModelPrivate
 
 class NetworkMediaModelPrivate
-    : public BaseModelPrivateT<NetworkMediaItemPtr>
-    , public LocalListCacheLoader<NetworkMediaItemPtr>::ModelSource
+    : public LocalListBaseModelPrivate<NetworkMediaItemPtr>
 {
     Q_DECLARE_PUBLIC(NetworkMediaModel)
 
 public:
     NetworkMediaModelPrivate(NetworkMediaModel* pub)
-        : BaseModelPrivateT<NetworkMediaItemPtr>(pub)
+        : LocalListBaseModelPrivate<NetworkMediaItemPtr>(pub)
         , m_preparseSem(1)
     {}
 
@@ -227,12 +225,7 @@ public:
 
     //BaseModelPrivateT interface implementation
 public:
-    bool loading() const override
-    {
-        return m_parsing || BaseModelPrivateT<NetworkMediaItemPtr>::loading();
-    }
-
-    LocalListCacheLoader<NetworkMediaItemPtr>::ItemCompare getSortFunction() const
+    LocalListCacheLoader<NetworkMediaItemPtr>::ItemCompare getSortFunction() const override
     {
         if (m_sortCriteria == "mrl" )
         {
@@ -288,12 +281,6 @@ public:
         }
     }
 
-    std::unique_ptr<ListCacheLoader<NetworkMediaItemPtr>> createLoader() const override
-    {
-        return std::make_unique<LocalListCacheLoader<NetworkMediaItemPtr>>(
-            this, m_searchPattern, getSortFunction());
-    }
-
     bool initializeModel() override
     {
         Q_Q(NetworkMediaModel);
@@ -384,9 +371,6 @@ public:
         m_preparseSem.acquire();
         vlc_media_tree_Preparse( tree, libvlc, q->m_treeItem.media.get(), this );
 
-        m_parsing = true;
-        emit q->loadingChanged();
-
         m_listener = std::move( l );
 
         return true;
@@ -394,12 +378,6 @@ public:
 
     // LocalListCacheLoader::ModelSource interface implementation
 protected:
-
-    size_t getModelRevision() const override
-    {
-        return m_revision;
-    }
-
     std::vector<NetworkMediaItemPtr> getModelData(const QString& pattern) const override
     {
         if (pattern.isEmpty())
@@ -415,12 +393,10 @@ protected:
     }
 
 public:
-    bool m_parsing = true;
     MediaLib* m_mediaLib;
     bool m_hasTree = false;
     QSemaphore m_preparseSem;
     std::unique_ptr<MediaTreeListener> m_listener;
-    size_t m_revision = 0;
     std::vector<NetworkMediaItemPtr> m_items;
 };
 
@@ -832,7 +808,7 @@ void NetworkMediaModel::ListenerCb::onItemPreparseEnded(MediaTreePtr, input_item
         if (p_node != model->m_treeItem.media)
             return;
 
-        model->d_func()->m_parsing = false;
+        model->d_func()->m_loading = false;
         emit model->loadingChanged();
     });
 }


=====================================
modules/gui/qt/network/networksourcesmodel.cpp
=====================================
@@ -20,8 +20,7 @@
 #include "networksourcesmodel.hpp"
 #include "networkmediamodel.hpp"
 
-#include "util/base_model_p.hpp"
-#include "util/locallistcacheloader.hpp"
+#include "util/locallistbasemodel.hpp"
 
 #include "playlist/media.hpp"
 #include "playlist/playlist_controller.hpp"
@@ -73,14 +72,13 @@ using SourceItemLists = std::vector<SourceItemPtr>;
 }
 
 class NetworkSourcesModelPrivate
-    : public BaseModelPrivateT<SourceItemPtr>
-    , public LocalListCacheLoader<SourceItemPtr>::ModelSource
+    : public LocalListBaseModelPrivate<SourceItemPtr>
 {
     Q_DECLARE_PUBLIC(NetworkSourcesModel);
 
 public: //Ctor/Dtor
     NetworkSourcesModelPrivate(NetworkSourcesModel* pub)
-        : BaseModelPrivateT<SourceItemPtr>(pub)
+        : LocalListBaseModelPrivate<SourceItemPtr>(pub)
     {}
 
 public:
@@ -94,7 +92,7 @@ public:
 
 public: // BaseModelPrivate implementation
 
-    LocalListCacheLoader<SourceItemPtr>::ItemCompare getSortFunction() const
+    LocalListCacheLoader<SourceItemPtr>::ItemCompare getSortFunction() const override
     {
         if (m_sortOrder == Qt::DescendingOrder)
             return [](const SourceItemPtr& a, const SourceItemPtr& b) {
@@ -116,12 +114,6 @@ public: // BaseModelPrivate implementation
             };
     }
 
-    std::unique_ptr<ListCacheLoader<SourceItemPtr>> createLoader() const override
-    {
-        return std::make_unique<LocalListCacheLoader<SourceItemPtr>>(
-            this, m_searchPattern, getSortFunction());
-    }
-
     bool initializeModel() override
     {
         Q_Q(NetworkSourcesModel);
@@ -153,15 +145,14 @@ public: // BaseModelPrivate implementation
             SourceItemPtr item = std::make_shared<SourceItem>(meta);
             m_items.push_back( item );
         }
-
+        //model is never udpdated but this is required to fit the LocalListBaseModelPrivate model
+        ++m_revision;
+        m_loading = false;
+        emit q->loadingChanged();
         return true;
     }
 
 public: // LocalListCacheLoader::ModelSource implementation
-    size_t getModelRevision() const override
-    {
-        return 1;
-    }
 
     std::vector<SourceItemPtr> getModelData(const QString& pattern) const override
     {


=====================================
modules/gui/qt/network/servicesdiscoverymodel.cpp
=====================================
@@ -18,8 +18,7 @@
 
 #include "servicesdiscoverymodel.hpp"
 
-#include "util/base_model_p.hpp"
-#include "util/locallistcacheloader.hpp"
+#include "util/locallistbasemodel.hpp"
 
 #include "medialibrary/mlhelper.hpp"
 
@@ -109,15 +108,15 @@ bool ListCache<SDItemPtr>::compareItems(const SDItemPtr& a, const SDItemPtr& b)
 // ServicesDiscoveryModelPrivate
 
 class ServicesDiscoveryModelPrivate
-    : public BaseModelPrivateT<SDItemPtr>
-    , public LocalListCacheLoader<SDItemPtr>::ModelSource
+    : public LocalListBaseModelPrivate<SDItemPtr>
 {
+
 public:
     Q_DECLARE_PUBLIC(ServicesDiscoveryModel)
 
 public: //ctor/dtor
     ServicesDiscoveryModelPrivate(ServicesDiscoveryModel* pub)
-        : BaseModelPrivateT<SDItemPtr>(pub)
+        : LocalListBaseModelPrivate<SDItemPtr>(pub)
     {
     }
 
@@ -139,19 +138,6 @@ public:
 public: //BaseModelPrivateT implementation
     bool initializeModel() override;
 
-    bool loading() const override
-    {
-        return m_loading || BaseModelPrivateT<SDItemPtr>::loading();
-    }
-
-    std::unique_ptr<ListCacheLoader<SDItemPtr>> createLoader() const override
-    {
-        return std::make_unique<LocalListCacheLoader<SDItemPtr>>(
-            this, m_searchPattern,
-            getSortFunction()
-            );
-    }
-
     LocalListCacheLoader<SDItemPtr>::ItemCompare getSortFunction() const
     {
         if (m_sortOrder == Qt::SortOrder::DescendingOrder)
@@ -172,12 +158,6 @@ public: //discovery callbacks
     void discoveryEnded();
 
 public: //LocalListCacheLoader implementation
-
-    size_t getModelRevision() const override
-    {
-        return m_revision;
-    }
-
     //return the data matching the pattern
     SDItemList getModelData(const QString& pattern) const override
     {
@@ -195,10 +175,7 @@ public: //LocalListCacheLoader implementation
     }
 
 public: // data
-    bool m_loading = true;
     addons_manager_t* m_manager = nullptr;
-
-    size_t m_revision = 0;
     SDItemList m_items;
 };
 
@@ -345,7 +322,6 @@ bool ServicesDiscoveryModelPrivate::initializeModel()
     m_manager = addons_manager_New( VLC_OBJECT( q->m_ctx->getIntf() ), &owner );
     assert( m_manager );
 
-    m_loading = true;
     emit q->loadingChanged();
     addons_manager_LoadCatalog( m_manager );
     addons_manager_Gather( m_manager, "repo://" );


=====================================
modules/gui/qt/network/standardpathmodel.cpp
=====================================
@@ -23,8 +23,7 @@
 #include "networkdevicemodel.hpp"
 #include "networkmediamodel.hpp"
 
-#include "util/base_model_p.hpp"
-#include "util/locallistcacheloader.hpp"
+#include "util/locallistbasemodel.hpp"
 #include "util/shared_input_item.hpp"
 
 // VLC includes
@@ -99,14 +98,13 @@ bool descendingMrl(const StandardPathItemPtr& a,
 
 
 class StandardPathModelPrivate
-    : public BaseModelPrivateT<StandardPathItemPtr>
-    , public LocalListCacheLoader<StandardPathItemPtr>::ModelSource
+    : public LocalListBaseModelPrivate<StandardPathItemPtr>
 {
     Q_DECLARE_PUBLIC(StandardPathModel)
 public:
 
     StandardPathModelPrivate(StandardPathModel* pub)
-        : BaseModelPrivateT<StandardPathItemPtr>(pub)
+        : LocalListBaseModelPrivate<StandardPathItemPtr>(pub)
     {}
 
     StandardPathLoader::ItemCompare getSortFunction() const
@@ -127,15 +125,9 @@ public:
         }
     }
 
-    std::unique_ptr<ListCacheLoader<StandardPathItemPtr>> createLoader() const override
-    {
-        return std::make_unique<StandardPathLoader>(
-            this, m_searchPattern,
-            getSortFunction());
-    }
-
     bool initializeModel() override
     {
+        Q_Q(StandardPathModel);
         assert(m_qmlInitializing == false);
 #ifdef Q_OS_UNIX
         addItem(QVLCUserDir(VLC_HOME_DIR), qtr("Home"), QUrl());
@@ -145,6 +137,10 @@ public:
         addItem(QVLCUserDir(VLC_MUSIC_DIR), qtr("Music"), QUrl());
         addItem(QVLCUserDir(VLC_VIDEOS_DIR), qtr("Videos"), QUrl());
         addItem(QVLCUserDir(VLC_DOWNLOAD_DIR), qtr("Download"), QUrl());
+        //model is never updated, but this is still needed to fit the LocalListBaseModelPrivate requirements
+        ++m_revision;
+        m_loading = false;
+        emit q->loadingChanged();
         return true;
     }
 
@@ -182,13 +178,6 @@ public:
     }
 
 public: //LocalListCacheLoader::ModelSource implementation
-
-    size_t getModelRevision() const override
-    {
-        //model never changes
-        return 1;
-    }
-
     std::vector<StandardPathItemPtr> getModelData(const QString& pattern) const override
     {
         if (pattern.isEmpty())


=====================================
modules/gui/qt/util/locallistbasemodel.hpp
=====================================
@@ -0,0 +1,73 @@
+/*****************************************************************************
+ * Copyright (C) 2024 VLC authors and VideoLAN
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * ( at your option ) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+#ifndef LOCALLISTBASEMODEL_HPP
+#define LOCALLISTBASEMODEL_HPP
+
+#include "locallistcacheloader.hpp"
+#include "base_model_p.hpp"
+
+/**
+ * @brief The LocalListBaseModelPrivate
+ * this ensure that cache is not created before data.
+ * If models have static data  or are synchronous, they still
+ * need to increase the mode revision to a non-zero value
+ */
+template<typename T>
+class  LocalListBaseModelPrivate
+    : public BaseModelPrivateT<T>
+    , public LocalListCacheLoader<T>::ModelSource
+{
+public:
+    using BaseModelPrivateT<T>::BaseModelPrivateT;
+    virtual ~LocalListBaseModelPrivate() = default;
+
+    virtual typename LocalListCacheLoader<T>::ItemCompare getSortFunction() const = 0;
+
+public:
+    //BaseModelPrivateT reimplementation
+    virtual bool loading() const override
+    {
+        return (m_loading) || BaseModelPrivateT<T>::loading();
+    }
+
+    virtual bool cachable() const override
+    {
+        return (m_revision > 0) && BaseModelPrivateT<T>::cachable();
+    }
+
+    virtual std::unique_ptr<ListCacheLoader<T>> createLoader() const override
+    {
+        return std::make_unique<LocalListCacheLoader<T>>(
+            this, this->m_searchPattern,
+            getSortFunction()
+            );
+    }
+
+public:
+    //LocalListCacheLoader<T>::ModelSource reimplementation
+    size_t getModelRevision() const override
+    {
+        return m_revision;
+    }
+
+protected:
+    size_t m_revision = 0;
+    bool m_loading = true;
+};
+
+#endif // LOCALLISTBASEMODEL_HPP



View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/2212ef872ee0a44d1ddd93d21da42a088a9c6499

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/2212ef872ee0a44d1ddd93d21da42a088a9c6499
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list