From 41cf43b820ebd549769c83e2b9624da0b910432e Mon Sep 17 00:00:00 2001 From: theGreatWhiteShark Date: Sat, 26 Oct 2024 16:34:57 +0200 Subject: [PATCH] AudioEngineTests: add offset test for JACK relocation The current state of transport implementation suffers a regression bug: whenever a tempo change occurs (tempo marker add/delete, BPM widget used...) `m_nFrameOffsetTempo` is altered. Since it is now handed to the JACK relocation callback as well, the resulting frame is off --- src/core/AudioEngine/AudioEngineTests.cpp | 354 ++++++++++++++---- src/core/AudioEngine/AudioEngineTests.h | 8 + .../h2JackTimebase/TransportTestsTimebase.cpp | 14 + .../h2JackTimebase/TransportTestsTimebase.h | 2 + 4 files changed, 306 insertions(+), 72 deletions(-) diff --git a/src/core/AudioEngine/AudioEngineTests.cpp b/src/core/AudioEngine/AudioEngineTests.cpp index d678eb3f2..f9b3c7d07 100644 --- a/src/core/AudioEngine/AudioEngineTests.cpp +++ b/src/core/AudioEngine/AudioEngineTests.cpp @@ -2250,88 +2250,204 @@ void AudioEngineTests::testTransportRelocationJack() { double fNewTick; long long nNewFrame; - auto waitForRelocation = [=]( double fTick, long long nFrame ) { - const int nMaxMilliSeconds = 5000; - const int nSecondTryMilliSeconds = 1000; - int nMilliSeconds = 0; - const int nIncrement = 100; - - // We send the tick value to and received it back from the JACK server - // via routines of the libjack2 library. We have to be relaxed about the - // precision we can expect from relocating. - while ( true ) { - - long long nCurrentFrame; - if ( pHydrogen->getJackTimebaseState() == + + double fTickMismatch; + const int nProcessCycles = 100; + for ( int nn = 0; nn < nProcessCycles; ++nn ) { + + // When being listener we have no way to properly check the resulting + // tick as our ground truth is just the frame information provided by + // the JACK server. + if ( pHydrogen->getJackTimebaseState() != JackAudioDriver::Timebase::Listener ) { - nCurrentFrame = pDriver->m_JackTransportPos.frame; + if ( nn < nProcessCycles - 2 ) { + fNewTick = tickDist( randomEngine ); + } + else if ( nn < nProcessCycles - 1 ) { + // Resulted in an unfortunate rounding error due to the + // song end. + fNewTick = pSong->lengthInTicks() - 1 + 0.928009209; } else { - nCurrentFrame = pTransportPos->getFrame() - - pTransportPos->getFrameOffsetTempo(); + // There was a rounding error at this particular tick. + fNewTick = std::fmin( 960, pSong->lengthInTicks() ); } - if ( ( nFrame != -1 && nFrame == nCurrentFrame ) || - ( fTick != -1 && - abs( pTransportPos->getDoubleTick() - fTick ) < 1e-1 ) ) { - return; + pAE->lock( RIGHT_HERE ); + + // Ensure we relocate to a new position. Else the assertions in this + // test will fail. + while ( std::abs( fNewTick - pTransportPos->getDoubleTick() ) < 1 ) { + fNewTick = tickDist( randomEngine ); } - if ( nMilliSeconds >= nMaxMilliSeconds ) { - QString sTarget; - if ( nFrame != -1 ) { - sTarget = QString( "frame [%1]" ).arg( nFrame ); - } else { - sTarget = QString( "tick [%1]" ).arg( fTick ); - } - AudioEngineTests::throwException( - QString( "[testTransportRelocationJack::waitForRelocation] playback takes too long to reach %1" ) - .arg( sTarget ) ); + INFOLOG( QString( "relocate to tick [%1]->[%2]" ) + .arg( pTransportPos->getDoubleTick() ).arg( fNewTick ) ); + pAE->locate( fNewTick, true ); + pAE->unlock(); + + AudioEngineTests::waitForRelocation( pDriver, fNewTick, -1 ); + // We send the tick value to and receive it from the JACK server via + // routines in the libjack2 library. We have to be relaxed about the + // precision we can expect. + if ( abs( pTransportPos->getDoubleTick() - fNewTick ) > 1e-1 ) { + throwException( QString( "[testTransportRelocationJack::tick] failed to relocate to tick. [%1] != [%2]" ) + .arg( pTransportPos->getDoubleTick() ).arg( fNewTick ) ); } - else if ( nMilliSeconds == nSecondTryMilliSeconds ) { - - WARNINGLOG( QString( "[testTransportRelocationJack::waitForRelocation] Performing seconds attempt after [%1]ms") - .arg( nMilliSeconds ) ); - - // Occassionally the JACK server seems to drop our relocation - // attempt silently. This is not good but acceptable since we - // are firing them in rapid succession. That's not the usual - // use-case. In order to not make this behavior break our test, - // we do a second attempt to be sure. - if ( fTick != -1 ) { - pAE->lock( RIGHT_HERE ); - pAE->locate( fTick, true ); - pAE->unlock(); - } - else { - pAE->lock( RIGHT_HERE ); #ifdef HAVE_INTEGRATION_TESTS - if ( pHydrogen->getJackTimebaseState() == - JackAudioDriver::Timebase::Listener ) { - // We are listener - // - // Discard the previous offset or we do not end up at - // the frame we provided to locateTransport and the - // comparison fails. - pDriver->m_nTimebaseFrameOffset = 0; - JackAudioDriver::m_nIntegrationLastRelocationFrame = -1; - } + // In case there is an issue with the BBT <-> transport position + // conversion or the m_nTimebaseFrameOffset, the driver will detect + // multiple relocations (maybe one in each cycle). + if ( pDriver->m_bIntegrationRelocationLoop ) { + throwException( "[testTransportRelocationJack::frame] relocation loop detected" ); + } #endif - pDriver->locateTransport( nFrame ); - pAE->unlock(); - } - } + AudioEngineTests::checkTransportPosition( + pTransportPos, "[testTransportRelocationJack::tick] mismatch tick-based" ); + } + + // Frame-based relocation + // We sample ticks and convert them since we are using tempo markers. + if ( nn < nProcessCycles - 1 ) { + nNewFrame = TransportPosition::computeFrameFromTick( + tickDist( randomEngine ), &fTickMismatch ); + } else { + // With this one there were rounding mishaps in v1.2.3 + nNewFrame = std::min( static_cast(2174246), + TransportPosition::computeFrameFromTick( + pSong->lengthInTicks(), &fTickMismatch ) ); + } + + pAE->lock( RIGHT_HERE ); - QTest::qSleep( nIncrement ); - nMilliSeconds += nIncrement; + while ( nNewFrame == pTransportPos->getFrame() ) { + nNewFrame = TransportPosition::computeFrameFromTick( + tickDist( randomEngine ), &fTickMismatch ); } - }; +#ifdef HAVE_INTEGRATION_TESTS + if ( pHydrogen->getJackTimebaseState() == + JackAudioDriver::Timebase::Listener ) { + // We are listener + // + // Discard the previous offset or we do not end up at the frame we + // provided to locateTransport and the comparison fails. + pDriver->m_nTimebaseFrameOffset = 0; + JackAudioDriver::m_nIntegrationLastRelocationFrame = -1; + } +#endif + + INFOLOG( QString( "relocate to frame [%1]->[%2]" ) + .arg( pTransportPos->getFrame() ).arg( nNewFrame ) ); + pDriver->locateTransport( nNewFrame ); + pAE->unlock(); + + AudioEngineTests::waitForRelocation( pDriver, -1, nNewFrame ); + + long long nCurrentFrame; + if ( pHydrogen->getJackTimebaseState() == + JackAudioDriver::Timebase::Listener ) { + nCurrentFrame = pDriver->m_JackTransportPos.frame; + } + else { + nCurrentFrame = pTransportPos->getFrame() - + pTransportPos->getFrameOffsetTempo(); + } + + if ( nNewFrame != nCurrentFrame ) { + throwException( QString( "[testTransportRelocationJack::frame] failed to relocate to frame. timebase state: [%1], nNewFrame [%2] != nCurrentFrame [%3], pPos->getFrame(): [%4], pPos->getFrameOffsetTempo: [%5]" ) + .arg( JackAudioDriver::TimebaseToQString( + pDriver->getTimebaseState() ) ) + .arg( nNewFrame ) + .arg( nCurrentFrame ) + .arg( pTransportPos->getFrame() ) + .arg( pTransportPos->getFrameOffsetTempo() ) ); + } + +#ifdef HAVE_INTEGRATION_TESTS + // In case there is an issue with the BBT <-> transport position + // conversion or the m_nTimebaseFrameOffset, the driver will detect + // multiple relocations (maybe one in each cycle). + if ( pDriver->m_bIntegrationRelocationLoop ) { + throwException( "[testTransportRelocationJack::frame] relocation loop detected" ); + } +#endif + + AudioEngineTests::checkTransportPosition( + pTransportPos, "[testTransportRelocationJack::frame] mismatch frame-based" ); + } + + pAE->lock( RIGHT_HERE ); +#ifdef HAVE_INTEGRATION_TESTS + pDriver->m_bIntegrationCheckRelocationLoop = false; + JackAudioDriver::m_nIntegrationLastRelocationFrame = -1; +#endif + pAE->reset( true ); + pAE->unlock(); + + stopJackAudioDriver(); +} + +void AudioEngineTests::testTransportRelocationOffsetsJack() { + auto pHydrogen = Hydrogen::get_instance(); + auto pSong = pHydrogen->getSong(); + auto pPref = Preferences::get_instance(); + auto pAE = pHydrogen->getAudioEngine(); + auto pTransportPos = pAE->getTransportPosition(); + + // When being JACK Timebase listener Hydrogen will adopt tempo setting + // provided by an external application and discards internal changes. This + // test would be identical to testTransportProcessingJack. + if ( pHydrogen->getJackTimebaseState() == + JackAudioDriver::Timebase::Listener ) { + return; + } + + CoreActionController::activateTimeline( false ); + + pAE->lock( RIGHT_HERE ); + pAE->stop(); + if ( pAE->getState() == AudioEngine::State::Playing ) { + pAE->stopPlayback(); + } + + // For this call the AudioEngine still needs to be in state + // Playing or Ready. + pAE->reset( true ); + pAE->unlock(); + + auto pDriver = startJackAudioDriver(); + if ( pDriver == nullptr ) { + throwException( "[testTransportRelocationOffsetsJack] Unable to use JACK driver" ); + } + float fBpm, fLastBpm; + bool bTempoChanged = false; + + pAE->lock( RIGHT_HERE ); + fLastBpm = pAE->getBpmAtColumn( 0 ); +#ifdef HAVE_INTEGRATION_TESTS + JackAudioDriver::m_nIntegrationLastRelocationFrame = -1; + pDriver->m_bIntegrationCheckRelocationLoop = true; +#endif + pAE->unlock(); + + std::random_device randomSeed; + std::default_random_engine randomEngine( randomSeed() ); + std::uniform_real_distribution tickDist( 0, pAE->m_fSongSizeInTicks ); + std::uniform_real_distribution tempoDist( MIN_BPM, MAX_BPM ); + + // Check consistency of updated frames and ticks while relocating + // transport. + double fNewTick; + long long nNewFrame; double fTickMismatch; const int nProcessCycles = 100; for ( int nn = 0; nn < nProcessCycles; ++nn ) { + if ( ! bTempoChanged && fLastBpm != pAE->getBpmAtColumn( 0 ) ) { + bTempoChanged = true; + } // When being listener we have no way to properly check the resulting // tick as our ground truth is just the frame information provided by @@ -2364,12 +2480,12 @@ void AudioEngineTests::testTransportRelocationJack() { pAE->locate( fNewTick, true ); pAE->unlock(); - waitForRelocation( fNewTick, -1 ); + AudioEngineTests::waitForRelocation( pDriver, fNewTick, -1 ); // We send the tick value to and receive it from the JACK server via // routines in the libjack2 library. We have to be relaxed about the // precision we can expect. if ( abs( pTransportPos->getDoubleTick() - fNewTick ) > 1e-1 ) { - throwException( QString( "[testTransportRelocationJack::tick] failed to relocate to tick. [%1] != [%2]" ) + throwException( QString( "[testTransportRelocationOffsetsJack::tick] failed to relocate to tick. [%1] != [%2]" ) .arg( pTransportPos->getDoubleTick() ).arg( fNewTick ) ); } @@ -2378,12 +2494,12 @@ void AudioEngineTests::testTransportRelocationJack() { // conversion or the m_nTimebaseFrameOffset, the driver will detect // multiple relocations (maybe one in each cycle). if ( pDriver->m_bIntegrationRelocationLoop ) { - throwException( "[testTransportRelocationJack::frame] relocation loop detected" ); + throwException( "[testTransportRelocationOffsetsJack::frame] relocation loop detected" ); } #endif AudioEngineTests::checkTransportPosition( - pTransportPos, "[testTransportRelocationJack::tick] mismatch tick-based" ); + pTransportPos, "[testTransportRelocationOffsetsJack::tick] mismatch tick-based" ); } // Frame-based relocation @@ -2422,7 +2538,7 @@ void AudioEngineTests::testTransportRelocationJack() { pDriver->locateTransport( nNewFrame ); pAE->unlock(); - waitForRelocation( -1, nNewFrame ); + AudioEngineTests::waitForRelocation( pDriver, -1, nNewFrame ); long long nCurrentFrame; if ( pHydrogen->getJackTimebaseState() == @@ -2435,7 +2551,7 @@ void AudioEngineTests::testTransportRelocationJack() { } if ( nNewFrame != nCurrentFrame ) { - throwException( QString( "[testTransportRelocationJack::frame] failed to relocate to frame. timebase state: [%1], nNewFrame [%2] != nCurrentFrame [%3], pPos->getFrame(): [%4], pPos->getFrameOffsetTempo: [%5]" ) + throwException( QString( "[testTransportRelocationOffsetsJack::frame] failed to relocate to frame. timebase state: [%1], nNewFrame [%2] != nCurrentFrame [%3], pPos->getFrame(): [%4], pPos->getFrameOffsetTempo: [%5]" ) .arg( JackAudioDriver::TimebaseToQString( pDriver->getTimebaseState() ) ) .arg( nNewFrame ) @@ -2449,12 +2565,22 @@ void AudioEngineTests::testTransportRelocationJack() { // conversion or the m_nTimebaseFrameOffset, the driver will detect // multiple relocations (maybe one in each cycle). if ( pDriver->m_bIntegrationRelocationLoop ) { - throwException( "[testTransportRelocationJack::frame] relocation loop detected" ); + throwException( "[testTransportRelocationOffsetsJack::frame] relocation loop detected" ); } #endif AudioEngineTests::checkTransportPosition( - pTransportPos, "[testTransportRelocationJack::frame] mismatch frame-based" ); + pTransportPos, "[testTransportRelocationOffsetsJack::frame] mismatch frame-based" ); + + fBpm = tempoDist( randomEngine ); + pAE->lock( RIGHT_HERE ); + INFOLOG( QString( "[testTransportRelocationOffsetsJack] changing tempo [%1]->[%2]" ) + .arg( pAE->getBpmAtColumn( 0 ) ).arg( fBpm ) ); + pAE->setNextBpm( fBpm ); + pAE->unlock(); + + AudioEngineTests::checkTransportPosition( + pTransportPos, "[testTransportRelocationOffsetsJack::tempo] mismatch after tempo change" ); } pAE->lock( RIGHT_HERE ); @@ -2465,6 +2591,10 @@ void AudioEngineTests::testTransportRelocationJack() { pAE->reset( true ); pAE->unlock(); + if ( ! bTempoChanged ) { + throwException( "[testTransportRelocationOffsetsJack] tempo was not change." ); + } + stopJackAudioDriver(); } @@ -2566,6 +2696,86 @@ void AudioEngineTests::stopJackAudioDriver() { INFOLOG( "DONE Stopping custom JACK audio driver." ); } +void AudioEngineTests::waitForRelocation( JackAudioDriver* pDriver, + double fTick, long long nFrame ) { + auto pHydrogen = Hydrogen::get_instance(); + auto pAE = pHydrogen->getAudioEngine(); + auto pTransportPos = pAE->getTransportPosition(); + + const int nMaxMilliSeconds = 5000; + const int nSecondTryMilliSeconds = 1000; + int nMilliSeconds = 0; + const int nIncrement = 100; + + // We send the tick value to and received it back from the JACK server + // via routines of the libjack2 library. We have to be relaxed about the + // precision we can expect from relocating. + while ( true ) { + long long nCurrentFrame; + if ( pHydrogen->getJackTimebaseState() == + JackAudioDriver::Timebase::Listener ) { + nCurrentFrame = pDriver->m_JackTransportPos.frame; + } else { + nCurrentFrame = pTransportPos->getFrame() - + pTransportPos->getFrameOffsetTempo(); + } + + if ( ( nFrame != -1 && nFrame == nCurrentFrame ) || + ( fTick != -1 && + abs( pTransportPos->getDoubleTick() - fTick ) < 1e-1 ) ) { + return; + } + + if ( nMilliSeconds >= nMaxMilliSeconds ) { + QString sTarget; + if ( nFrame != -1 ) { + sTarget = QString( "frame [%1]" ).arg( nFrame ); + } else { + sTarget = QString( "tick [%1]" ).arg( fTick ); + } + AudioEngineTests::throwException( + QString( "[AudioEngineTests::waitForRelocation] playback takes too long to reach %1" ) + .arg( sTarget ) ); + } else if ( nMilliSeconds == nSecondTryMilliSeconds ) { + WARNINGLOG( QString( "[AudioEngineTests::waitForRelocation] Performing seconds attempt after [%1]ms" ) + .arg( nMilliSeconds ) ); + + // Occassionally the JACK server seems to drop our relocation + // attempt silently. This is not good but acceptable since we + // are firing them in rapid succession. That's not the usual + // use-case. In order to not make this behavior break our test, + // we do a second attempt to be sure. + if ( fTick != -1 ) { + pAE->lock( RIGHT_HERE ); + pAE->locate( fTick, true ); + pAE->unlock(); + } + else { + pAE->lock( RIGHT_HERE ); + +#ifdef HAVE_INTEGRATION_TESTS + if ( pHydrogen->getJackTimebaseState() == + JackAudioDriver::Timebase::Listener ) { + // We are listener + // + // Discard the previous offset or we do not end up at + // the frame we provided to locateTransport and the + // comparison fails. + pDriver->m_nTimebaseFrameOffset = 0; + JackAudioDriver::m_nIntegrationLastRelocationFrame = -1; + } +#endif + + pDriver->locateTransport( nFrame ); + pAE->unlock(); + } + } + + QTest::qSleep( nIncrement ); + nMilliSeconds += nIncrement; + } +} + int AudioEngineTests::jackTestProcessCallback( uint32_t nframes, void* args ) { AudioEngine* pAudioEngine = Hydrogen::get_instance()->getAudioEngine(); diff --git a/src/core/AudioEngine/AudioEngineTests.h b/src/core/AudioEngine/AudioEngineTests.h index 2910d76bc..fbf56e46c 100644 --- a/src/core/AudioEngine/AudioEngineTests.h +++ b/src/core/AudioEngine/AudioEngineTests.h @@ -117,6 +117,12 @@ class AudioEngineTests : public H2Core::Object * audioEngine_process() using the JACK audio driver. */ static void testTransportRelocationJack(); + /** + * Unit test similar to testTransportRelocationJack() but is altering tempo + * and song size during playback as well in order to test the handling of + * all frame offsets. + */ + static void testTransportRelocationOffsetsJack(); /** Process callback for the testing instance of the * #H2Core::JackAudioDriver */ @@ -178,6 +184,8 @@ class AudioEngineTests : public H2Core::Object #ifdef H2CORE_HAVE_JACK static void stopJackAudioDriver(); + static void waitForRelocation( JackAudioDriver* pDriver, + double fTick, long long nFrame ); #endif }; }; diff --git a/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.cpp b/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.cpp index c03b5eae3..9869bea5c 100644 --- a/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.cpp +++ b/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.cpp @@ -77,6 +77,20 @@ void TransportTestsTimebase::testTransportRelocationJack() { ___INFOLOG( "\npassed\n" ); } +void TransportTestsTimebase::testTransportRelocationOffsetsJack() { + ___INFOLOG( "\n\n" ); +#ifdef H2CORE_HAVE_JACK + auto pHydrogen = Hydrogen::get_instance(); + auto pPref = Preferences::get_instance(); + + perform( &AudioEngineTests::testTransportRelocationOffsetsJack ); +#else + throw CppUnit::Exception( "Unapplicable. Compiled without JACK support!" ); +#endif + + ___INFOLOG( "\npassed\n" ); +} + void TransportTestsTimebase::perform( std::function func ) { try { func(); diff --git a/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.h b/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.h index ee9ad680a..555332eb3 100644 --- a/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.h +++ b/tests/jackTimebase/h2JackTimebase/TransportTestsTimebase.h @@ -32,12 +32,14 @@ class TransportTestsTimebase : public CppUnit::TestCase { CPPUNIT_TEST( testTransportProcessingJack ); CPPUNIT_TEST( testTransportProcessingOffsetsJack ); CPPUNIT_TEST( testTransportRelocationJack ); + CPPUNIT_TEST( testTransportRelocationOffsetsJack ); CPPUNIT_TEST_SUITE_END(); public: void testTransportProcessingJack(); void testTransportProcessingOffsetsJack(); void testTransportRelocationJack(); + void testTransportRelocationOffsetsJack(); virtual void tearDown() override { // The tests in here tend to produce a very large number of log