Improve thread safety in UI code

- Implemented better C++ classes for handling scenes/sources/items in
  obs.hpp, allowing them to automatically increment and decrement the
  references of each, as well as assign them to QVariants.

- Because QVariants are now using the C++ classes, remove the pointer
  QVariant wrapper.

- Use the new C++ classes with the QVariant user data of list box items,
  both for the sake of thread safety and to ensure that the data
  referenced is not freed until removed.  NOTE: still might need some
  testing.

- Implemented a source-remove signal from libobs, and start using that
  signal instead of the source-destroy signal for signalling item
  removal.

libobs/obs-scene.c

@@ -199,19 +199,17 @@ obs_scene_t obs_scene_create(const char *name)
 
 int obs_scene_addref(obs_scene_t scene)
 {
-	return obs_source_addref(scene->source);
+	return scene ? obs_source_addref(scene->source) : 0;
 }
 
 int obs_scene_release(obs_scene_t scene)
 {
-	if (scene)
-		return obs_source_release(scene->source);
-	return 0;
+	return scene ? obs_source_release(scene->source) : 0;
 }
 
 obs_source_t obs_scene_getsource(obs_scene_t scene)
 {
-	return scene->source;
+	return scene ? scene->source : NULL;
 }
 
 obs_scene_t obs_scene_fromsource(obs_source_t source)

libobs/obs-source.c

@@ -250,20 +250,23 @@ void obs_source_remove(obs_source_t source)
 
 	pthread_mutex_lock(&data->sources_mutex);
 
-	if (!source)
+	if (!source || source->removed)
 		return;
 
-	if (!source->removed) {
-		source->removed = true;
+	source->removed = true;
 
-		id = da_find(data->sources, &source, 0);
-		if (id != DARRAY_INVALID) {
-			da_erase_item(data->sources, &source);
-			obs_source_release(source);
-		}
+	obs_source_addref(source);
+
+	id = da_find(data->sources, &source, 0);
+	if (id != DARRAY_INVALID) {
+		da_erase_item(data->sources, &source);
+		obs_source_release(source);
 	}
 
 	pthread_mutex_unlock(&data->sources_mutex);
+
+	obs_source_dosignal(source, "source-remove");
+	obs_source_release(source);
 }
 
 bool obs_source_removed(obs_source_t source)

libobs/obs.hpp

@@ -26,75 +26,71 @@
 
 /* RAII wrappers */
 
-class OBSSource {
-	obs_source_t source;
-
-	OBSSource(OBSSource &&) = delete;
-	OBSSource(OBSSource const&) = delete;
-
-	OBSSource &operator=(OBSSource const&) = delete;
-
-public:
-	inline OBSSource(obs_source_t source) : source(source) {}
-	inline ~OBSSource() {obs_source_release(source);}
-
-	inline OBSSource& operator=(obs_source_t p) {source = p; return *this;}
-
-	inline operator obs_source_t() {return source;}
-
-	inline bool operator==(obs_source_t p) const {return source == p;}
-	inline bool operator!=(obs_source_t p) const {return source != p;}
-};
-
-class OBSSourceRef {
-	obs_source_t source;
+template<class RefClass> class OBSRef {
+	typedef typename RefClass::type T;
+	T val;
 
 public:
-	inline OBSSourceRef(obs_source_t source) : source(source)
-	{
-		obs_source_addref(source);
-	}
+	inline OBSRef() : val(nullptr)                  {}
+	inline OBSRef(T val_) : val(val_)               {RefClass::AddRef(val);}
+	inline OBSRef(const OBSRef &ref) : val(ref.val) {RefClass::AddRef(val);}
+	inline OBSRef(OBSRef &&ref) : val(ref.val)      {ref.val = nullptr;}
 
-	inline OBSSourceRef(const OBSSourceRef &ref) : source(ref.source)
-	{
-		obs_source_addref(source);
-	}
+	inline ~OBSRef() {RefClass::Release(val);}
 
-	inline OBSSourceRef(OBSSourceRef &&ref) : source(ref.source)
+	inline OBSRef &operator=(T valIn)
 	{
-		ref.source = NULL;
-	}
-
-	inline ~OBSSourceRef() {obs_source_release(source);}
-
-	inline OBSSourceRef &operator=(obs_source_t sourceIn)
-	{
-		obs_source_addref(sourceIn);
-		obs_source_release(source);
-		source = sourceIn;
+		RefClass::AddRef(valIn);
+		RefClass::Release(val);
+		val = valIn;
 		return *this;
 	}
 
-	inline OBSSourceRef &operator=(const OBSSourceRef &ref)
+	inline OBSRef &operator=(const OBSRef &ref)
 	{
-		obs_source_addref(ref.source);
-		obs_source_release(source);
-		source = ref.source;
+		RefClass::AddRef(ref.val);
+		RefClass::Release(val);
+		val = ref.val;
 		return *this;
 	}
 
-	inline OBSSourceRef &operator=(OBSSourceRef &&ref)
+	inline OBSRef &operator=(OBSRef &&ref)
 	{
 		if (this != &ref) {
-			source = ref.source;
-			ref.source = NULL;
+			val = ref.val;
+			ref.val = NULL;
 		}
 
 		return *this;
 	}
 
-	inline operator obs_source_t() const {return source;}
+	inline operator T() const {return val;}
 
-	inline bool operator==(obs_source_t p) const {return source == p;}
-	inline bool operator!=(obs_source_t p) const {return source != p;}
+	inline bool operator==(T p) const {return val == p;}
+	inline bool operator!=(T p) const {return val != p;}
 };
+
+class OBSSourceRefClass {
+public:
+	typedef obs_source_t type;
+	static inline void AddRef(type val)  {obs_source_addref(val);}
+	static inline void Release(type val) {obs_source_release(val);}
+};
+
+class OBSSceneRefClass {
+public:
+	typedef obs_scene_t type;
+	static inline void AddRef(type val)  {obs_scene_addref(val);}
+	static inline void Release(type val) {obs_scene_release(val);}
+};
+
+class OBSSceneItemRefClass {
+public:
+	typedef obs_sceneitem_t type;
+	static inline void AddRef(type val)  {obs_sceneitem_addref(val);}
+	static inline void Release(type val) {obs_sceneitem_release(val);}
+};
+
+typedef OBSRef<OBSSourceRefClass>    OBSSource;
+typedef OBSRef<OBSSceneRefClass>     OBSScene;
+typedef OBSRef<OBSSceneItemRefClass> OBSSceneItem;

obs/qt-ptr-variant.hpp

@@ -1,37 +0,0 @@
-/******************************************************************************
-    Copyright (C) 2013 by Hugh Bailey <[email protected]>
-
-    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, see <http://www.gnu.org/licenses/>.
-******************************************************************************/
-
-#include <QVariant>
-
-/* in case you're wondering, I made this code because qt static asserts would
- * fail on forward pointers.  so I was forced to make a hack */
-
-struct PtrVariantDummy {
-	void *unused;
-};
-
-Q_DECLARE_METATYPE(PtrVariantDummy*);
-
-template<typename T> static inline T VariantPtr(QVariant v)
-{
-	return (T)v.value<PtrVariantDummy*>();
-}
-
-static inline QVariant PtrVariant(void* ptr)
-{
-	return QVariant::fromValue<PtrVariantDummy*>((PtrVariantDummy*)ptr);
-}

obs/window-basic-main.cpp

@@ -25,24 +25,24 @@
 #include "window-namedialog.hpp"
 #include "window-basic-main.hpp"
 #include "qt-wrappers.hpp"
-#include "qt-ptr-variant.hpp"
 
 #include "ui_OBSBasic.h"
 
 using namespace std;
 
-obs_scene_t OBSBasic::GetCurrentScene()
+Q_DECLARE_METATYPE(OBSScene);
+Q_DECLARE_METATYPE(OBSSceneItem);
+
+OBSScene OBSBasic::GetCurrentScene()
 {
 	QListWidgetItem *item = ui->scenes->currentItem();
-	return item ? VariantPtr<obs_scene_t>(item->data(Qt::UserRole)) :
-		nullptr;
+	return item ? item->data(Qt::UserRole).value<OBSScene>() : nullptr;
 }
 
-obs_sceneitem_t OBSBasic::GetCurrentSceneItem()
+OBSSceneItem OBSBasic::GetCurrentSceneItem()
 {
 	QListWidgetItem *item = ui->sources->currentItem();
-	return item ? VariantPtr<obs_sceneitem_t>(item->data(Qt::UserRole)) :
-		nullptr;
+	return item ? item->data(Qt::UserRole).value<OBSSceneItem>() : nullptr;
 }
 
 void OBSBasic::AddScene(obs_source_t source)
@@ -51,7 +51,7 @@ void OBSBasic::AddScene(obs_source_t source)
 	obs_scene_t scene = obs_scene_fromsource(source);
 
 	QListWidgetItem *item = new QListWidgetItem(QT_UTF8(name));
-	item->setData(Qt::UserRole, PtrVariant(scene));
+	item->setData(Qt::UserRole, QVariant::fromValue(OBSScene(scene)));
 	ui->scenes->addItem(item);
 
 	signal_handler_t handler = obs_source_signalhandler(source);
@@ -77,13 +77,14 @@ void OBSBasic::RemoveScene(obs_source_t source)
 
 void OBSBasic::AddSceneItem(obs_sceneitem_t item)
 {
-	obs_scene_t scene = obs_sceneitem_getscene(item);
+	obs_scene_t  scene  = obs_sceneitem_getscene(item);
 	obs_source_t source = obs_sceneitem_getsource(item);
-	const char *name = obs_source_getname(source);
+	const char   *name  = obs_source_getname(source);
 
 	if (GetCurrentScene() == scene) {
 		QListWidgetItem *listItem = new QListWidgetItem(QT_UTF8(name));
-		listItem->setData(Qt::UserRole, PtrVariant(item));
+		listItem->setData(Qt::UserRole,
+				QVariant::fromValue(OBSSceneItem(item)));
 
 		ui->sources->insertItem(0, listItem);
 	}
@@ -100,7 +101,7 @@ void OBSBasic::RemoveSceneItem(obs_sceneitem_t item)
 			QListWidgetItem *listItem = ui->sources->item(i);
 			QVariant userData = listItem->data(Qt::UserRole);
 
-			if (item == VariantPtr<obs_sceneitem_t>(userData)) {
+			if (userData.value<OBSSceneItem>() == item) {
 				delete listItem;
 				break;
 			}
@@ -185,7 +186,7 @@ void OBSBasic::SourceAdded(void *data, calldata_t params)
 		static_cast<OBSBasic*>(data)->AddScene(source);
 }
 
-void OBSBasic::SourceDestroyed(void *data, calldata_t params)
+void OBSBasic::SourceRemoved(void *data, calldata_t params)
 {
 	obs_source_t source = (obs_source_t)calldata_ptr(params, "source");
 
@@ -229,8 +230,8 @@ void OBSBasic::OBSInit()
 
 	signal_handler_connect(obs_signalhandler(), "source-add",
 			OBSBasic::SourceAdded, this);
-	signal_handler_connect(obs_signalhandler(), "source-destroy",
-			OBSBasic::SourceDestroyed, this);
+	signal_handler_connect(obs_signalhandler(), "source-remove",
+			OBSBasic::SourceRemoved, this);
 	signal_handler_connect(obs_signalhandler(), "channel-change",
 			OBSBasic::ChannelChanged, this);
 
@@ -246,6 +247,8 @@ void OBSBasic::OBSInit()
 
 OBSBasic::~OBSBasic()
 {
+	ui->sources->clear();
+	ui->scenes->clear();
 	obs_shutdown();
 }
 
@@ -364,7 +367,7 @@ void OBSBasic::on_scenes_itemChanged(QListWidgetItem *item)
 	if (item) {
 		obs_scene_t scene;
 
-		scene = VariantPtr<obs_scene_t>(item->data(Qt::UserRole));
+		scene = item->data(Qt::UserRole).value<OBSScene>();
 		source = obs_scene_getsource(scene);
 		UpdateSources(scene);
 	}
@@ -413,7 +416,7 @@ void OBSBasic::on_actionRemoveScene_triggered()
 		return;
 
 	QVariant userData = item->data(Qt::UserRole);
-	obs_scene_t scene = VariantPtr<obs_scene_t>(userData);
+	obs_scene_t scene = userData.value<OBSScene>();
 	obs_source_t source = obs_scene_getsource(scene);
 	obs_source_remove(source);
 }

obs/window-basic-main.hpp

@@ -32,8 +32,8 @@ class OBSBasic : public OBSMainWindow {
 private:
 	std::unordered_map<obs_source_t, int> sourceSceneRefs;
 
-	obs_scene_t GetCurrentScene();
-	obs_sceneitem_t GetCurrentSceneItem();
+	OBSScene GetCurrentScene();
+	OBSSceneItem GetCurrentSceneItem();
 	void AddSceneItem(obs_sceneitem_t item);
 	void RemoveSceneItem(obs_sceneitem_t item);
 	void AddScene(obs_source_t scene);
@@ -45,7 +45,7 @@ private:
 	static void SceneItemAdded(void *data, calldata_t params);
 	static void SceneItemRemoved(void *data, calldata_t params);
 	static void SourceAdded(void *data, calldata_t params);
-	static void SourceDestroyed(void *data, calldata_t params);
+	static void SourceRemoved(void *data, calldata_t params);
 	static void ChannelChanged(void *data, calldata_t params);
 
 	void ResizePreview(uint32_t cx, uint32_t cy);
@@ -87,7 +87,7 @@ private slots:
 
 public:
 	explicit OBSBasic(QWidget *parent = 0);
-	~OBSBasic();
+	virtual ~OBSBasic();
 
 	virtual void OBSInit() override;