File Fix-driver-switching-whil-using-JACK.patch of Package hydrogen

Overview Repositories Revisions Requests Users Attributes Meta

File Fix-driver-switching-whil-using-JACK.patch of Package hydrogen (Revision 1dbbd6792f41e8c0efa965f8b9e98a86)

Currently displaying revision 1dbbd6792f41e8c0efa965f8b9e98a86 , Show latest

From d822aeddc5d43821d92a2ecea8fd3baf58e5dc5b Mon Sep 17 00:00:00 2001
From: theGreatWhiteShark <princess.trudildis@posteo.de>
Date: Thu, 14 Dec 2023 22:04:03 +0100
Subject: [PATCH 1/4] tests: add AudioDriverTest

a unit tests which switches between all available drivers and "checks" whether Hydrogen crashes in between.

Which drivers are properly created does of course depend on the OS, installed packages etc. But it is a starting point and helps a lot during debugging
---
 src/tests/AudioDriverTest.cpp | 65 +++++++++++++++++++++++++++++++++++
 src/tests/AudioDriverTest.h   | 39 +++++++++++++++++++++
 src/tests/registeredTests.h   |  2 ++
 3 files changed, 106 insertions(+)
 create mode 100644 src/tests/AudioDriverTest.cpp
 create mode 100644 src/tests/AudioDriverTest.h

diff --git a/src/tests/AudioDriverTest.cpp b/src/tests/AudioDriverTest.cpp
new file mode 100644
index 0000000000..3dd44e4431
--- /dev/null
+++ b/src/tests/AudioDriverTest.cpp
@@ -0,0 +1,65 @@
+/*
+ * Hydrogen
+ * Copyright(c) 2008-2023 The hydrogen development team [hydrogen-devel@lists.sourceforge.net]
+ *
+ * http://www.hydrogen-music.org
+ *
+ * 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 https://www.gnu.org/licenses
+ *
+ */
+
+#include "AudioDriverTest.h"
+
+#include <core/AudioEngine/AudioEngine.h>
+#include <core/Hydrogen.h>
+
+void AudioDriverTest::testDriverSwitching() {
+	___INFOLOG("");
+
+	auto pHydrogen = H2Core::Hydrogen::get_instance();
+	auto pAudioEngine = pHydrogen->getAudioEngine();
+
+	for ( int ii = 0; ii < 10; ++ii ) {
+		std::cout << ii << std::endl;
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "ALSA" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "OSS" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "JACK" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "PortAudio" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "CoreAudio" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "PulseAudio" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "DiskWriterDriver" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "NullDriver" );
+		pAudioEngine->stopAudioDrivers();
+		pAudioEngine->createAudioDriver( "Fake" );
+	}
+
+	___INFOLOG("done");
+}
+
+void AudioDriverTest::tearDown() {
+	auto pHydrogen = H2Core::Hydrogen::get_instance();
+	auto pAudioEngine = pHydrogen->getAudioEngine();
+
+	pAudioEngine->stopAudioDrivers();
+	pAudioEngine->createAudioDriver( "Fake" );
+
+}
diff --git a/src/tests/AudioDriverTest.h b/src/tests/AudioDriverTest.h
new file mode 100644
index 0000000000..4ece54bae9
--- /dev/null
+++ b/src/tests/AudioDriverTest.h
@@ -0,0 +1,39 @@
+/*
+ * Hydrogen
+ * Copyright(c) 2008-2023 The hydrogen development team [hydrogen-devel@lists.sourceforge.net]
+ *
+ * http://www.hydrogen-music.org
+ *
+ * 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 https://www.gnu.org/licenses
+ *
+ */
+
+#ifndef AUDIO_DRIVER_TEST_H
+#define AUDIO_DRIVER_TEST_H
+
+#include <cppunit/extensions/HelperMacros.h>
+
+class AudioDriverTest : public CppUnit::TestCase {
+	CPPUNIT_TEST_SUITE( AudioDriverTest );
+	CPPUNIT_TEST( testDriverSwitching );
+	CPPUNIT_TEST_SUITE_END();
+
+	public:
+		virtual void tearDown();
+
+		// Check that drivers can be switched without any crashes.
+		void testDriverSwitching();
+};
+
+#endif
diff --git a/src/tests/registeredTests.h b/src/tests/registeredTests.h
index 5718d03ce2..e93ac6bcb9 100644
--- a/src/tests/registeredTests.h
+++ b/src/tests/registeredTests.h
@@ -23,6 +23,7 @@
 #include <cppunit/extensions/HelperMacros.h>
 
 #include "AdsrTest.h"
+#include "AudioDriverTest.h"
 #include "AutomationPathSerializerTest.cpp"
 #include "AutomationPathTest.cpp"
 #include "CoreActionControllerTest.h"
@@ -42,6 +43,7 @@
 #include "XmlTest.h"
 
 CPPUNIT_TEST_SUITE_REGISTRATION( ADSRTest );
+CPPUNIT_TEST_SUITE_REGISTRATION( AudioDriverTest );
 CPPUNIT_TEST_SUITE_REGISTRATION( AutomationPathSerializerTest );
 CPPUNIT_TEST_SUITE_REGISTRATION( AutomationPathTest );
 CPPUNIT_TEST_SUITE_REGISTRATION( CoreActionControllerTest );

From c0cb947f429cdd95ac4755d2f380b9b32c8c5e15 Mon Sep 17 00:00:00 2001
From: theGreatWhiteShark <princess.trudildis@posteo.de>
Date: Thu, 14 Dec 2023 22:12:53 +0100
Subject: [PATCH 2/4] AudioEngine: fix crash on mutex locking

for some reason Hydrogen crashes when attempting to audio engine mutex dedicated to the output buffers while switching/stopping drivers.

Thread 37 "tests" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffd1722700 (LWP 4178)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff6717537 in __GI_abort () at abort.c:79
#2  0x00007ffff6ac37ec in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff6ace966 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff6ace9d1 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff6ace3cc in __gxx_personality_v0 () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff68d98a4 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#7  0x00007ffff68d9f4e in _Unwind_ForcedUnwind () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#8  0x00007ffff79f5c30 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:121
#9  0x00007ffff79ea729 in __do_cancel () at ./pthreadP.h:310
#10 sigcancel_handler (sig=32, si=0x7fffd1721330, ctx=<optimized out>) at nptl-init.c:177
#11 sigcancel_handler (sig=<optimized out>, si=0x7fffd1721330, ctx=<optimized out>) at nptl-init.c:142
#12 <signal handler called>
#13 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#14 0x00007ffff6cc2f05 in QBasicMutex::lockInternal() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x00007ffff7d354dc in QMutexLocker::QMutexLocker (this=0x7fffd1721908, m=0x5555557489d8) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qmutex.h:233
#16 0x00007ffff7d25be6 in H2Core::AudioEngine::clearAudioBuffers (this=0x555555748950, nFrames=32) at /home/phil/git/hydrogen-1.2/src/core/AudioEngine/AudioEngine.cpp:786
#17 0x00007ffff7d29223 in H2Core::AudioEngine::audioEngine_process (nframes=32) at /home/phil/git/hydrogen-1.2/src/core/AudioEngine/AudioEngine.cpp:1318
#18 0x00007ffff64eac8f in Jack::JackClient::CallProcessCallback() () from /usr/local/lib/libjack.so.0
#19 0x00007ffff64eab96 in Jack::JackClient::ExecuteThread() () from /usr/local/lib/libjack.so.0
#20 0x00007ffff64e8641 in Jack::JackClient::Execute() () from /usr/local/lib/libjack.so.0
#21 0x00007ffff6508484 in Jack::JackPosixThread::ThreadHandler(void*) () from /usr/local/lib/libjack.so.0
#22 0x00007ffff79ebea7 in start_thread (arg=<optimized out>) at pthread_create.c:477
#23 0x00007ffff67f0a2f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

replacing the calls to `QMutexLocker` with raw `.lock()` one of the mutex itself fixes the problem. This might be a Qt bug (but I have not found anything similar in their bug tracker).

I also have to note that I encountered quite a number of crashes of the JACK server itself while switching drivers hundreds of times during debugging (on board sound card and minimal buffer size). But none of them seems to be related to Hydrogen. I will report them upstream.
---
 ChangeLog                            |  2 ++
 src/core/AudioEngine/AudioEngine.cpp | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index dbd3d4c950..dcf5a4ef6a 100644
diff --git a/src/core/AudioEngine/AudioEngine.cpp b/src/core/AudioEngine/AudioEngine.cpp
index 8cba97631f..128d09cead 100644
--- a/src/core/AudioEngine/AudioEngine.cpp
+++ b/src/core/AudioEngine/AudioEngine.cpp
@@ -783,7 +783,7 @@ void AudioEngine::calculateTransportOffsetOnBpmChange( std::shared_ptr<Transport
 
 void AudioEngine::clearAudioBuffers( uint32_t nFrames )
 {
-	QMutexLocker mx( &m_MutexOutputPointer );
+	m_MutexOutputPointer.lock();
 	float *pBuffer_L, *pBuffer_R;
 
 	// clear main out Left and Right
@@ -805,7 +805,7 @@ void AudioEngine::clearAudioBuffers( uint32_t nFrames )
 	}
 #endif
 
-	mx.unlock();
+	m_MutexOutputPointer.unlock();
 
 #ifdef H2CORE_HAVE_LADSPA
 	if ( getState() == State::Ready ||
@@ -889,7 +889,7 @@ AudioOutput* AudioEngine::createAudioDriver( const QString& sDriver )
 	}
 
 	this->lock( RIGHT_HERE );
-	QMutexLocker mx(&m_MutexOutputPointer);
+	m_MutexOutputPointer.lock();
 
 	// Some audio drivers require to be already registered in the
 	// AudioEngine while being connected.
@@ -903,7 +903,7 @@ AudioOutput* AudioEngine::createAudioDriver( const QString& sDriver )
 
 	// Unlocking earlier might execute the jack process() callback before we
 	// are fully initialized.
-	mx.unlock();
+	m_MutexOutputPointer.unlock();
 	this->unlock();
 	
 	nRes = m_pAudioDriver->connect();
@@ -913,12 +913,12 @@ AudioOutput* AudioEngine::createAudioDriver( const QString& sDriver )
 				  .arg( sDriver ).arg( nRes ) );
 
 		this->lock( RIGHT_HERE );
-		mx.relock();
+		m_MutexOutputPointer.lock();
 		
 		delete m_pAudioDriver;
 		m_pAudioDriver = nullptr;
 		
-		mx.unlock();
+		m_MutexOutputPointer.unlock();
 		this->unlock();
 
 		return nullptr;
@@ -978,7 +978,7 @@ void AudioEngine::startAudioDrivers()
 	}
 
 	this->lock( RIGHT_HERE );
-	QMutexLocker mx(&m_MutexOutputPointer);
+	m_MutexOutputPointer.lock();
 	
 	if ( pPref->m_sMidiDriver == "ALSA" ) {
 #ifdef H2CORE_HAVE_ALSA
@@ -1014,7 +1014,7 @@ void AudioEngine::startAudioDrivers()
 #endif
 	}
 	
-	mx.unlock();
+	m_MutexOutputPointer.unlock();
 	this->unlock();
 }
 
@@ -1048,10 +1048,10 @@ void AudioEngine::stopAudioDrivers()
 
 	if ( m_pAudioDriver != nullptr ) {
 		m_pAudioDriver->disconnect();
-		QMutexLocker mx( &m_MutexOutputPointer );
+		m_MutexOutputPointer.lock();
 		delete m_pAudioDriver;
 		m_pAudioDriver = nullptr;
-		mx.unlock();
+		m_MutexOutputPointer.unlock();
 	}
 
 	this->unlock();

From f2620ce4027274de0d7c2db0a9933f5b3f8fe11e Mon Sep 17 00:00:00 2001
From: theGreatWhiteShark <princess.trudildis@posteo.de>
Date: Wed, 20 Dec 2023 15:38:19 +0100
Subject: [PATCH 3/4] Update AudioDriverTest.h

tests: proper setup and tear down

of AudioDriverTest. Switching the audio drivers round and round does overwrite the buffer size setting done in the main routine before triggering the unit test. These have to be reapplied
---
 src/tests/AudioDriverTest.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/tests/AudioDriverTest.h b/src/tests/AudioDriverTest.h
index 4ece54bae9..3b448d4f73 100644
--- a/src/tests/AudioDriverTest.h
+++ b/src/tests/AudioDriverTest.h
@@ -24,16 +24,23 @@
 
 #include <cppunit/extensions/HelperMacros.h>
 
+#include <QString>
+
 class AudioDriverTest : public CppUnit::TestCase {
 	CPPUNIT_TEST_SUITE( AudioDriverTest );
 	CPPUNIT_TEST( testDriverSwitching );
 	CPPUNIT_TEST_SUITE_END();
 
 	public:
+		virtual void setUp();
 		virtual void tearDown();
 
 		// Check that drivers can be switched without any crashes.
 		void testDriverSwitching();
+
+	private:
+		int m_nPrevBufferSize;
+		QString m_sPrevAudioDriver;
 };
 
 #endif

From 1c58d9c26e8febde2b0161cf7585be71c9e17729 Mon Sep 17 00:00:00 2001
From: theGreatWhiteShark <princess.trudildis@posteo.de>
Date: Wed, 20 Dec 2023 15:40:27 +0100
Subject: [PATCH 4/4] Update AudioDriverTest.cpp

tests: proper setup and tear down

diff --git a/src/tests/AudioDriverTest.cpp b/src/tests/AudioDriverTest.cpp
index 3dd44e4431..b43cf073ec 100644
--- a/src/tests/AudioDriverTest.cpp
+++ b/src/tests/AudioDriverTest.cpp
@@ -23,6 +23,13 @@
 
 #include <core/AudioEngine/AudioEngine.h>
 #include <core/Hydrogen.h>
+#include <core/Preferences/Preferences.h>
+
+void AudioDriverTest::setUp() {
+	auto pPref = H2Core::Preferences::get_instance();
+	m_nPrevBufferSize = pPref->m_nBufferSize;
+	m_sPrevAudioDriver = pPref->m_sAudioDriver;
+}
 
 void AudioDriverTest::testDriverSwitching() {
 	___INFOLOG("");
@@ -59,7 +66,10 @@ void AudioDriverTest::tearDown() {
 	auto pHydrogen = H2Core::Hydrogen::get_instance();
 	auto pAudioEngine = pHydrogen->getAudioEngine();
 
+	auto pPref = H2Core::Preferences::get_instance();
+	pPref->m_nBufferSize = m_nPrevBufferSize;
+	pPref->m_sAudioDriver = m_sPrevAudioDriver;
+
 	pAudioEngine->stopAudioDrivers();
 	pAudioEngine->createAudioDriver( "Fake" );
-
 }

Places

File Fix-driver-switching-whil-using-JACK.patch of Package hydrogen (Revision 1dbbd6792f41e8c0efa965f8b9e98a86)

Places