Blob Blame History Raw
From 6edddf7c8e92bb499cb60fdee53622cab326f334 Mon Sep 17 00:00:00 2001
From: Christian Esken <esken@kde.org>
Date: Tue, 2 Jul 2013 00:44:51 +0200
Subject: [PATCH 2/4] Revert my direct use of
 ControlManager::instance().announce(). PA actually did call it in its own
 thread and so in the end we have a non-GUI thread calling GUI code. Use
 invokeMethod() again. CCBUGS: 309464

---
 backends/mixer_pulse.cpp | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 backends/mixer_pulse.h   |  7 +++++++
 2 files changed, 54 insertions(+)

diff --git a/backends/mixer_pulse.cpp b/backends/mixer_pulse.cpp
index e08a881..5202b5a 100644
--- a/backends/mixer_pulse.cpp
+++ b/backends/mixer_pulse.cpp
@@ -769,9 +769,55 @@ static devmap* get_widget_map(int type, int index)
 
 void Mixer_PULSE::emitControlsReconfigured()
 {
+	//	emit controlsReconfigured(_mixer->id());
+    // Do not emit directly to ensure all connected slots are executed
+    // in their own event loop.
+
+	/*
+	 * Comment by cesken: I am not really sure what the comment above means.
+	 *  1) IIRC coling told me "otherwise KMix crashes".
+	 *  2) There are also bug reports that heavily indicate the crash when operation the "move stream" from a popup
+	 *     menu.
+	 *  3) I don't know what the "executed in their own event loop" means. Are we in a "wrong" thread here (PA),
+	 *     which is not suitable for GUI code?!?
+	 *
+	 *  Conclusions:
+	 *  a) It seems there seems to be some object deletion hazard with a QMenu (the one for "move stream")
+	 *  b)  I do not see why executing it Queued is better, because you can never know when it is actually being
+	 *      executed: it could be "right now". It looks like Qt currently executes it after the QMenu hazard has
+	 *      resolved itselves miracously.
+	 *  c) I am definitely strongly opposed on this "execute later" approach. It is pure gambling IMO and might be
+	 *     broken any time (from DEBUG to RELEASE build, or by a new Qt or KDE version).
+	 *
+	 *     TODO Somebody with more Qt and PA internal insight might help to clear up things here.
+	 *
+	 *  Temporary solution: Do the QueuedConnection until we really know hat is going on. But the called code
+	 *                      pulseControlsReconfigured() will then do the standard announce() so that every part of
+	 *                      KMix automatically gets updated.
+	 *
+	 */
+    QMetaObject::invokeMethod(this,
+                              "pulseControlsReconfigured",
+                              Qt::QueuedConnection);
+
+//    QMetaObject::invokeMethod(this,
+//                              "pulseControlsReconfigured",
+//                              Qt::QueuedConnection,
+//                              Q_ARG(QString, _mixer->id()));
+}
+
+void Mixer_PULSE::pulseControlsReconfigured()
+{
+	kDebug() << "Reconfigure " << _mixer->id();
     ControlManager::instance().announce(_mixer->id(), ControlChangeType::ControlList, getDriverName());
 }
 
+void Mixer_PULSE::pulseControlsReconfigured(QString mixerId)
+{
+	kDebug() << "Reconfigure " << mixerId;
+    ControlManager::instance().announce(mixerId, ControlChangeType::ControlList, getDriverName());
+}
+
 void Mixer_PULSE::addWidget(int index, bool isAppStream)
 {
     devmap* map = get_widget_map(m_devnum, index);
@@ -1366,3 +1412,4 @@ QString Mixer_PULSE::getDriverName()
         return "PulseAudio";
 }
 
+#include "mixer_pulse.moc"
diff --git a/backends/mixer_pulse.h b/backends/mixer_pulse.h
index ac65534..ef2050c 100644
--- a/backends/mixer_pulse.h
+++ b/backends/mixer_pulse.h
@@ -45,6 +45,8 @@ typedef struct {
 
 class Mixer_PULSE : public Mixer_Backend
 {
+    Q_OBJECT
+
     public:
         Mixer_PULSE(Mixer *mixer, int devnum);
         virtual ~Mixer_PULSE();
@@ -77,6 +79,11 @@ class Mixer_PULSE : public Mixer_Backend
         void addDevice(devinfo& dev, bool = false);
         bool connectToDaemon();
         void emitControlsReconfigured();
+
+   protected slots:
+        void pulseControlsReconfigured(QString mixerId);
+        void pulseControlsReconfigured();
+
 public:
         void reinit();
 
-- 
1.8.3.1