hydrogen: remove JACK-audio-driver patch

It did not enhance nothing for me

Signed-off-by: Andreas Müller <schnitzeltony@gmail.com>
This commit is contained in:
Andreas Müller
2020-02-27 20:58:09 +01:00
parent 6c81fcaf1b
commit 3fd01d0a72
2 changed files with 0 additions and 511 deletions

View File

@@ -1,510 +0,0 @@
From 28d45a275212f9a28ce4bfe343a89bd2cb58cab8 Mon Sep 17 00:00:00 2001
From: theGreatWhiteShark <thetruephil@googlemail.com>
Date: Sun, 9 Dec 2018 15:04:38 +0100
Subject: [PATCH] Fixing bugs in the JACK audio driver
1. the *beat*, *bar*, and *tick* information got misinterpreted as the
current transport position propagated by the external JACK timebase
master. But a "tick" is an arbitrary quantization of the beat and
the last one passed at the current transport position will be
provided by the BBT information of the JACK server. Therefore it
almost always lags behind current transport and thus triggering a
warning described in issue 602.
I rewrote the code in such a way the if clause probing for a
discrepancy between the current transport position + the
`bbt_frame_offset` and the JACK server transport position is only
entered after relocations of the JACK transport position. It will
set the current transport position to the one of the server and
resets `bbt_frame_offset` to zero. The corner case of an external
JACK timebase master changing the tempo using the BBT information
and some relocations taking place afterwards will be covered by
keeping track of the tick size upon during tempo change and
remapping the position of the relocation to our internal version
with a different speed.
I'm not 100% sure what the *beat*, *bar*, *tick* information are
actually provided for. Everything works flawless at my end and no
one of the jack-devel mailing list could/wanted to give me any
example or use case my implementation would not be able to handle.
2. The internal transport position and the `bbt_frame_offset` variable
were not properly reset after a relocation by the user took place.
Upstream-Status: Submitted [1]
[1] https://github.com/hydrogen-music/hydrogen/pull/678/commits/28d45a275212f9a28ce4bfe343a89bd2cb58cab8
---
.../include/hydrogen/IO/jack_audio_driver.h | 81 +++----
src/core/src/IO/jack_audio_driver.cpp | 228 +++++-------------
2 files changed, 92 insertions(+), 217 deletions(-)
diff --git a/src/core/include/hydrogen/IO/jack_audio_driver.h b/src/core/include/hydrogen/IO/jack_audio_driver.h
index 4ac94be6..8bfeab08 100644
--- a/src/core/include/hydrogen/IO/jack_audio_driver.h
+++ b/src/core/include/hydrogen/IO/jack_audio_driver.h
@@ -122,7 +122,6 @@ public:
* Constructor of the JACK server driver.
*
* Sets a number of variables:
- * - #must_relocate = 0
* - #locate_countdown = 0
* - #bbt_frame_offset = 0
* - #track_port_count = 0
@@ -487,16 +486,7 @@ public:
or _JackTransportStarting_, TransportInfo::m_status will
be set to TransportInfo::STOPPED. If its
_JackTransportRolling_, TransportInfo::m_status will be
- set to TransportInfo::ROLLING and in case
- TransportInfo::m_status was not in this state beforehand,
- #must_relocate will be set to 2. This will trigger a
- relocation in relocateBBT() based on the updated
- information of the JACK timebase master (not ourselves) at
- the end of updateTransportInfo() during the _next_
- cycle. We have to wait one cycle since the client being
- the current JACK timebase master also just started playing
- and it needs this cycle to update the bar, beat, and tick
- information.
+ set to TransportInfo::ROLLING.
* - Calls Hydrogen::setTimelineBpm() to update both the
global speed of the Song Song::__bpm, as well as the
fallback speed Hydrogen::m_nNewBpmJTM() with the local
@@ -507,37 +497,28 @@ public:
instance of the TransportInfo
AudioOutput::m_transport. The tempo will be set by the
calling function audioEngine_process_transport()
- afterwards. In addition, #must_relocate will be set to 1
- in order to relocate using relocateBBT() at the end of
- the function call. After all we learned that the time
- master applied some changes. So let's be sure things are
- set right.
+ afterwards.
* - In case the transport position changed due to an user
interaction (e.g. clicking somewhere at the Timeline) or
a relocation triggered by another JACK client, we will
- detect this change since position stored in
+ detect this change since the position stored in
TransportInfo::m_nFrames plus the constant offset
#bbt_frame_offset does not equate to the _frame_ member
- of #m_JackTransportPos anymore. Then we will check
- whether another JACK timebase master is present and set
- #must_relocate to 2 if the timebase master did not provided a
- different speed beforehand. So, we wait one cycle for him
- to update all information and call relocateBBT()
- afterwards. If Hydrogen is the JACK timebase master itself,
- TransportInfo::m_nFrames is set to its value of the
- previous cycle by assigned the #m_nHumantimeFrames. If,
- on the other hand, no Jack timebase master is present at all,
- TransportInfo::m_nFrames is set to the current position
- of the JACK server (_frame_ component of
- #m_JackTransportPos), #bbt_frame_offset is reset to 0,
- and Hydrogen::triggerRelocateDuringPlay() is called if
- the transport is rolling.
+ of #m_JackTransportPos anymore. It there is a timebase
+ master present and it is us or no timebase master was set
+ at all, #bbt_frame_offset will be reset to zero and
+ TransportInfo::m_nFrames will be set to the _frame_ member
+ of #m_JackTransportPos. The relocation itself will be
+ handled by locate() and we trust the JACK server to do
+ its job properly. If there is an external timebase
+ master, a change in speed (or relocation into a region of
+ different speed) could have occurred. Therefore,
+ TransportInfo::m_nFrames will be scaled properly using
+ #m_fOldTickSize and TransportInfo::m_nTickSize. In addition,
+ Hydrogen::triggerRelocateDuringPlay() will be called if
+ no timebase master is present and the transport is rolling.
* - Finally, #m_nHumantimeFrames will be set to the _frame_
- member of #m_JackTransportPos, relocateBBT() called if
- #must_relocate equals one,
- Hydrogen::triggerRelocateDuringPlay() be called if the
- transport is also rolling, and #must_relocate decremented
- by one.
+ member of #m_JackTransportPos.
*
* If Preferences::USE_JACK_TRANSPORT was not selected in
* Preferences::m_bJackTransportMode, the function will return
@@ -715,16 +696,13 @@ protected:
private:
/**
- * Updates the tick size TransportInfo::m_nTickSize and frame
- * position TransportInfo::m_nFrames using the transport
- * position information obtained from another JACK timebase master.
+ * Internal tick size used before a tempo change.
*
- * It is only entered via updateTransportInfo() if there is
- * JACK timebase master and returns without performing any actions
- * if Hydrogen itself is the timebase master.
+ * This variable will only be used in the very particular
+ * border case of having an external JACK timebase master,
+ * which did update the tempo, and one of the clients
*/
- void relocateBBT();
-
+ float m_fOldTickSize;
/**
* Constant offset between the transport position in
* TransportInfo::m_nFrames and the one obtained by the JACK
@@ -740,15 +718,6 @@ private:
* updated in calculateFrameOffset().
*/
long long bbt_frame_offset;
- /** Triggering relocateBBT() in #must_relocate -1 cycles at
- the end of updateTransportInfo().
- *
- * This is either done immediately if another JACK timebase
- * master (not Hydrogen itself!) changed the speed or during
- * the next cycle if the other timebase master changed the
- * transport position.
- */
- int must_relocate;
/**
* #locate_countdown - 1 of cycles (calls to audioEngine_process())
* until to locate() to #locate_frame in
@@ -906,6 +875,12 @@ private:
* used to initialize this variable.
*/
bool m_bConnectOutFlag;
+ /**
+ * Whether Hydrogen is the current Jack timebase master.
+ *
+ * It gets initialized in init().
+ */
+ bool m_bHydrogenIsJackTimebaseMaster;
/**
* Specifies whether to use a conditional take over in the
* switching of the JACK timebase master. If set to non-zero
diff --git a/src/core/src/IO/jack_audio_driver.cpp b/src/core/src/IO/jack_audio_driver.cpp
index 767fb1fa..bdd23c8d 100644
--- a/src/core/src/IO/jack_audio_driver.cpp
+++ b/src/core/src/IO/jack_audio_driver.cpp
@@ -167,11 +167,10 @@ JackAudioDriver::JackAudioDriver( JackProcessCallback processCallback )
pJackDriverInstance = this;
this->processCallback = processCallback;
- must_relocate = 0;
locate_countdown = 0;
bbt_frame_offset = 0;
track_port_count = 0;
-
+
memset( track_output_ports_L, 0, sizeof(track_output_ports_L) );
memset( track_output_ports_R, 0, sizeof(track_output_ports_R) );
}
@@ -198,7 +197,6 @@ int JackAudioDriver::connect()
return 1;
}
-
bool connect_output_ports = m_bConnectOutFlag;
memset( track_output_ports_L, 0, sizeof(track_output_ports_L) );
@@ -322,95 +320,6 @@ void JackAudioDriver::locateInNCycles( unsigned long frame, int cycles_to_wait )
locate_countdown = cycles_to_wait;
locate_frame = frame;
}
-
-void JackAudioDriver::relocateBBT()
-{
- Preferences* pPref = Preferences::get_instance();
-
- // If Hydrogen itself is the JACK timebase master, this
- // function is not relevant.
- if ( m_transport.m_status != TransportInfo::ROLLING
- || pPref->m_bJackMasterMode == Preferences::USE_JACK_TIME_MASTER
- || ! ( m_JackTransportPos.valid & JackPositionBBT )
- ) {
- // WARNINGLOG( "Relocate: Call it off" );
- return;
- }
-
- // WARNINGLOG( "Relocate..." );
-
- Hydrogen * H = Hydrogen::get_instance();
- Song * S = H->getSong();
-
- // m_JackTransportPos.beat_type contains the denominator of
- // the time signature and is given in number of ticks per
- // quarter. Thus, hydrogen_TPB contains the number of beats
- // per denominator of the JACK time signature. (Ticks per bar)
- float hydrogen_TPB = ( float )( S->__resolution / m_JackTransportPos.beat_type * 4 );
-
- long bar_ticks = 0;
- //long beat_ticks = 0;
- if ( S->get_mode() == Song::SONG_MODE ) {
- // m_JackTransportPos.bar accesses the current bar at
- // the transport position. (Reasonable?) assumption
- // that one pattern is _always_ 1 bar long!
- bar_ticks = H->getTickForPosition( m_JackTransportPos.bar-1 );
- // If a tick corresponding to the position of the
- // current bar-1 could not be found, we will use the
- // tick at the very beginning of the song instead.
- if ( bar_ticks < 0 ) bar_ticks = 0;
- }
-
- // m_JackTransportPos.beat refers to the current beat within a
- // bar, m_JackTransportPos.tick to the current tick within a
- // beat, and m_JackTransportPos.ticks_per_beat to the number
- // of ticks per beat.
- // Using hydrogen_TPB as conversion factor, the relocation
- // position in ticks will be calculated by combining the
- // current bar, beat, and tick.
- float hydrogen_ticks_to_locate = bar_ticks +
- ( m_JackTransportPos.beat-1 ) * hydrogen_TPB +
- m_JackTransportPos.tick * ( hydrogen_TPB / m_JackTransportPos.ticks_per_beat );
-
- INFOLOG( QString( "Position from Timebase Master: BBT [%1,%2,%3]" ).arg( m_JackTransportPos.bar ).arg( m_JackTransportPos.beat ).arg( m_JackTransportPos.tick ) );
- // WARNINGLOG( "Tx/Beat = "+to_string(m_JackTransportPos.ticks_per_beat)+", Meter "+to_string(m_JackTransportPos.beats_per_bar)+"/"+to_string(m_JackTransportPos.beat_type)+" =>tick " + to_string( hydrogen_ticks_to_locate ) );
-
- // Before calling this function in updateTransportInfo() the
- // speed obtained by the JACK server query was already written
- // to m_transport.m_nBPM. This value can thus be considered
- // clean.
- float fNewTickSize = getSampleRate() * 60.0 / m_transport.m_nBPM / S->__resolution;
-
- if ( fNewTickSize == 0 ) return;
-
- // Note that this will prevent
- // audioEngine_process_checkBPMChanged() in Hydrogen.cpp from
- // recalculating things unless m_transport.m_nBpm and S->__bpm
- // aren't equal.
- m_transport.m_nTickSize = fNewTickSize;
-
- // New transport position in frames
- long long nNewFrames = ( long long )( hydrogen_ticks_to_locate * fNewTickSize );
-
-#ifndef JACK_NO_BBT_OFFSET
- // m_JackTransportPos.bbt_offset is a frame offset for the BBT
- // fields (the given bar, beat, and tick values actually refer
- // to a time frame_offset frames before the start of the
- // cycle), should be assumed to be 0 if JackBBTFrameOffset is
- // not set. If JackBBTFrameOffset is set and this value is
- // zero, the BBT time refers to the first frame of this
- // cycle. If the value is positive, the BBT time refers to a
- // frame that many frames before the start of the cycle.
- //
- // Due to the if clause in the beginning of the function, the
- // following if clause is true at all times.
- if ( m_JackTransportPos.valid & JackBBTFrameOffset )
- nNewFrames += m_JackTransportPos.bbt_offset;
-#endif
-
- m_transport.m_nFrames = nNewFrames;
-}
-
void JackAudioDriver::updateTransportInfo()
{
// The following four lines do only cover the special case of
@@ -453,21 +362,8 @@ void JackAudioDriver::updateTransportInfo()
break;
case JackTransportRolling: // Transport is playing
- // If the valid member matches the JackPositionBBT
- // bits, there is a JACK timebase master present
- // supplying bar, beat, and tick information along
- // with the plain transport position in frames. We
- // will use it for re-positioning. But only at the end
- // of the next cycle, because this timebase master has
- // to properly initialize its state during this cycle
- // too.
- if ( m_transport.m_status != TransportInfo::ROLLING &&
- ( m_JackTransportPos.valid & JackPositionBBT ) ) {
- must_relocate = 2;
- //WARNINGLOG( "Jack transport starting: Resyncing in 2 x Buffersize!!" );
- }
m_transport.m_status = TransportInfo::ROLLING;
- //INFOLOG( "[updateTransportInfo] ROLLING - frames: " + to_string(m_transportPos.frame) );
+ //INFOLog( "[updateTransportInfo] ROLLING - frames: " + to_string(m_transportPos.frame) );
break;
case JackTransportStarting: // Waiting for sync ready. If
@@ -503,9 +399,14 @@ void JackAudioDriver::updateTransportInfo()
if ( m_transport.m_nBPM != bpm ) {
if ( Preferences::get_instance()->m_bJackMasterMode ==
Preferences::NO_JACK_TIME_MASTER ){
- // WARNINGLOG( QString( "Tempo change from jack-transport: %1" ).arg( bpm ) );
- m_transport.m_nBPM = bpm;
- must_relocate = 1;
+ // The speed of the Song will be updated by function
+ // calling update_transport_info()
+ // audioEngine_process_transport().
+ m_transport.m_nBPM = bpm;
+
+ // The tick size will be updated by the
+ // audioEngine_process_checkBPMChanged()
+ // function afterwards.
}
}
}
@@ -515,37 +416,41 @@ void JackAudioDriver::updateTransportInfo()
// button or clicking somewhere on the timeline) or by a
// different JACK client.
if ( m_transport.m_nFrames + bbt_frame_offset != m_JackTransportPos.frame ) {
- // Whether there is a JACK timebase master registered
- // but the audio engine was neither (re)started nor a
- // change in the speed was found beforehand.
- if ( ( m_JackTransportPos.valid & JackPositionBBT ) && must_relocate == 0 ) {
- // WARNINGLOG( "Frame offset mismatch; triggering resync in 2 cycles" );
- must_relocate = 2; // re-locate at the end of
- // the next cycle.
- } else {
- if ( Preferences::get_instance()->m_bJackMasterMode ==
- Preferences::NO_JACK_TIME_MASTER ) {
- // Just consider the transport
- // position as dirty, reassign the
- // one obtained in the query, and
- // reset the offset.
- m_transport.m_nFrames = m_JackTransportPos.frame;
+ INFOLOG( "A relocation took place" );
+ if ( ( m_JackTransportPos.valid & JackPositionBBT ) ){
+ // There is a JACK timebase master.
+ if ( !m_bHydrogenIsJackTimebaseMaster ){
+ // But it's not us.
+ // Absorbing the current bbt_frame_offset value.
+ m_transport.m_nFrames = m_JackTransportPos.frame *
+ m_fOldTickSize/ m_transport.m_nTickSize;
bbt_frame_offset = 0;
- // If not registered as JACK time
- // master, the following line is
- // needed to be able to relocate by
- // clicking the song ruler (wired
- // corner case, but still...)
- if ( m_transport.m_status == TransportInfo::ROLLING )
- // Setting
- // H->m_nPatternStartTick = -1
- // if in pattern mode.
- H->triggerRelocateDuringPlay();
+ // INFOLOG( QString( "External JACK timebase master. Resetting frame from %1 to %2 using the old tick size %3 and new one %4" )
+ // .arg( m_transport.m_nFrames )
+ // .arg( m_JackTransportPos.frame )
+ // .arg( m_fOldTickSize )
+ // .arg( m_transport.m_nTickSize ) );
} else {
- // Fallback to the transport position
- // of the last cycle.
- m_transport.m_nFrames = H->getHumantimeFrames();
+ // INFOLOG( QString( "Hydrogen as JACK timebase master. Possible mismatch between transport %1, client %2, bbt_frame_offset %3" )
+ // .arg( m_transport.m_nFrames )
+ // .arg( m_JackTransportPos.frame )
+ // .arg( bbt_frame_offset ) );
+ // We are timebase master ourselves.
+ // Resetting the offset.
+ bbt_frame_offset = 0;
+ // We do not update the position in frames directly.
+ // Instead, the JACK server is asked to relocate
+ // and we use its current location.
+ m_transport.m_nFrames = m_JackTransportPos.frame;
}
+ } else {
+ // Only normal JACK clients connected
+ // to the server.
+ // INFOLOG( "Relocation as normal JACK client" );
+ m_transport.m_nFrames = m_JackTransportPos.frame;
+ bbt_frame_offset = 0;
+ if ( m_transport.m_status == TransportInfo::ROLLING )
+ H->triggerRelocateDuringPlay();
}
}
// Only used if Hydrogen is in JACK timebase master
@@ -554,20 +459,6 @@ void JackAudioDriver::updateTransportInfo()
H->setHumantimeFrames(m_JackTransportPos.frame);
// WARNINGLOG( QString( "fix Humantime: %1" ).arg( m_JackTransportPos.frame ) );
}
-
- // Trigger the re-location based on the information provided
- // by another JACK timebase master.
- if ( must_relocate == 1 ) {
- WARNINGLOG( QString( "Resyncing! bbt_frame_offset: %1" ).arg( bbt_frame_offset ) );
- relocateBBT();
- if ( m_transport.m_status == TransportInfo::ROLLING ) {
- // Setting H->m_nPatternStartTick = -1 if in
- // pattern mode
- H->triggerRelocateDuringPlay();
- }
- }
-
- if ( must_relocate > 0 ) must_relocate--;
}
float* JackAudioDriver::getOut_L()
@@ -609,7 +500,7 @@ float* JackAudioDriver::getTrackOut_R( unsigned nTrack )
if( p ) {
out = (jack_default_audio_sample_t*) jack_port_get_buffer( p, jack_server_bufferSize);
}
- return out;
+ return out;
}
float* JackAudioDriver::getTrackOut_L( Instrument * instr, InstrumentComponent * pCompo)
@@ -623,20 +514,20 @@ float* JackAudioDriver::getTrackOut_R( Instrument * instr, InstrumentComponent *
}
-#define CLIENT_FAILURE(msg) { \
- ERRORLOG("Could not connect to JACK server (" msg ")"); \
- if (m_pClient) { \
- ERRORLOG("...but JACK returned a non-null pointer?"); \
- (m_pClient) = nullptr; \
-} \
- if (nTries) ERRORLOG("...trying again."); \
+#define CLIENT_FAILURE(msg) { \
+ ERRORLOG("Could not connect to JACK server (" msg ")"); \
+ if (m_pClient) { \
+ ERRORLOG("...but JACK returned a non-null pointer?"); \
+ (m_pClient) = nullptr; \
+ } \
+ if (nTries) ERRORLOG("...trying again."); \
}
-#define CLIENT_SUCCESS(msg) { \
- assert(m_pClient); \
- INFOLOG(msg); \
- nTries = 0; \
+#define CLIENT_SUCCESS(msg) { \
+ assert(m_pClient); \
+ INFOLOG(msg); \
+ nTries = 0; \
}
int JackAudioDriver::init( unsigned bufferSize )
@@ -842,6 +733,8 @@ int JackAudioDriver::init( unsigned bufferSize )
m_nJackConditionalTakeOver = 0;
// Make Hydrogen the JACK timebase master.
initTimeMaster();
+ } else {
+ m_bHydrogenIsJackTimebaseMaster = false;
}
return 0;
@@ -1130,12 +1023,18 @@ void JackAudioDriver::initTimeMaster()
// - other non-zero error code.
int ret = jack_set_timebase_callback(m_pClient, m_nJackConditionalTakeOver,
jack_timebase_callback, this);
- if (ret != 0) pref->m_bJackMasterMode = Preferences::NO_JACK_TIME_MASTER;
+ if (ret != 0){
+ pref->m_bJackMasterMode = Preferences::NO_JACK_TIME_MASTER;
+ m_bHydrogenIsJackTimebaseMaster = false;
+ } else {
+ m_bHydrogenIsJackTimebaseMaster = true;
+ }
} else {
// Called by the timebase master to release itself
// from that responsibility (defined in
// jack/transport.h).
jack_release_timebase(m_pClient);
+ m_bHydrogenIsJackTimebaseMaster = false;
}
}
@@ -1144,6 +1043,7 @@ void JackAudioDriver::com_release()
if ( m_pClient == nullptr) return;
jack_release_timebase(m_pClient);
+ m_bHydrogenIsJackTimebaseMaster = false;
}
void JackAudioDriver::jack_timebase_callback(jack_transport_state_t state,
--
2.21.0

View File

@@ -36,7 +36,6 @@ DEPENDS += " \
SRC_URI = " \
git://github.com/hydrogen-music/hydrogen.git \
file://0001-hydrogen.default.conf-do-not-show-developer-warnings.patch \
file://0002-Fixing-bugs-in-the-JACK-audio-driver.patch \
\
http://hydrogen-music.org/feeds/drumkit_list.php;name=drumkit-list \
\