From 6b9c8e79cc2da0ca6bf63b13a2519b75ce2dd891 Mon Sep 17 00:00:00 2001 From: David Bailes Date: Wed, 15 Jan 2020 11:12:40 +0000 Subject: [PATCH] Bug 1954: Clicks may occur starting/pausing play-at-speed or Scrub Problem: On Windows, after 50ms, there is a short period of roughly zero introduced into the output. On Linux, there is also a spike which sounds like a crackle. In AudioIO::FillBuffers(), Mixer::SetTimesAndSpeed() is called, which sets mT0 and mT1 to a small interval. In Mixer::MixVariableRates(), all the samples in the interval are used, which means the Resample::Process() is called with last equal to true. So when Mixer::MixVariableRates() is called again, the resampler is being reused after a call to Process() in which last is true. It is not stated in the soxr documentation if the resampler will produce valid results in this case, and it's only the scrubbing code which does this. I think this is the problem, and so the partial fix below avoids this happening. Partial fix for play-at-speed and keyboard scrubbing: For these, there is no need to reset the values of mT0 and mT1. (There is no need to allow for the sample position being used to potentially jump around.) So for these cases, Mixer::SetSpeed() is called, rather than Mixer::SetTimesAndSpeed(). --- src/AudioIO.cpp | 14 +++++++++++--- src/AudioIOBase.cpp | 12 ++++++++---- src/AudioIOBase.h | 4 +++- src/Mix.cpp | 6 ++++++ src/Mix.h | 1 + src/tracks/ui/Scrubbing.cpp | 3 +++ 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/AudioIO.cpp b/src/AudioIO.cpp index 96096123f..0e43e67ba 100644 --- a/src/AudioIO.cpp +++ b/src/AudioIO.cpp @@ -2795,6 +2795,7 @@ void AudioIO::FillBuffers() #ifdef EXPERIMENTAL_SCRUBBING_SUPPORT case PlaybackSchedule::PLAY_SCRUB: case PlaybackSchedule::PLAY_AT_SPEED: + case PlaybackSchedule::PLAY_KEYBOARD_SCRUB: { mScrubDuration -= frames; wxASSERT(mScrubDuration >= 0); @@ -2825,9 +2826,16 @@ void AudioIO::FillBuffers() double(diff) / mScrubDuration.as_double(); if (!mSilentScrub) { - for (i = 0; i < mPlaybackTracks.size(); i++) - mPlaybackMixers[i]->SetTimesAndSpeed( - startTime, endTime, fabs( mScrubSpeed )); + for (i = 0; i < mPlaybackTracks.size(); i++) { + if (mPlaybackSchedule.mPlayMode == PlaybackSchedule::PLAY_AT_SPEED || + mPlaybackSchedule.mPlayMode == PlaybackSchedule::PLAY_KEYBOARD_SCRUB) { + mPlaybackMixers[i]->SetSpeed(mScrubSpeed); + } + else { + mPlaybackMixers[i]->SetTimesAndSpeed( + startTime, endTime, fabs( mScrubSpeed )); + } + } } mTimeQueue.mLastTime = startTime; } diff --git a/src/AudioIOBase.cpp b/src/AudioIOBase.cpp index ea34a1df6..c65f393e5 100644 --- a/src/AudioIOBase.cpp +++ b/src/AudioIOBase.cpp @@ -427,10 +427,14 @@ void AudioIOBase::PlaybackSchedule::Init( wxASSERT(false); scrubbing = false; } - else - mPlayMode = (scrubOptions.isPlayingAtSpeed) - ? PlaybackSchedule::PLAY_AT_SPEED - : PlaybackSchedule::PLAY_SCRUB; + else { + if (scrubOptions.isPlayingAtSpeed) + mPlayMode = PLAY_AT_SPEED; + else if (scrubOptions.isKeyboardScrubbing) + mPlayMode = PLAY_KEYBOARD_SCRUB; + else + mPlayMode = PLAY_SCRUB; + } } #endif diff --git a/src/AudioIOBase.h b/src/AudioIOBase.h index 1586c9206..4b138fbce 100644 --- a/src/AudioIOBase.h +++ b/src/AudioIOBase.h @@ -49,6 +49,7 @@ struct ScrubbingOptions { bool bySpeed {}; bool isPlayingAtSpeed{}; + bool isKeyboardScrubbing{}; double delay {}; @@ -358,6 +359,7 @@ protected: #ifdef EXPERIMENTAL_SCRUBBING_SUPPORT PLAY_SCRUB, PLAY_AT_SPEED, // a version of PLAY_SCRUB. + PLAY_KEYBOARD_SCRUB, #endif } mPlayMode { PLAY_STRAIGHT }; double mCutPreviewGapStart; @@ -411,7 +413,7 @@ protected: bool PlayingStraight() const { return mPlayMode == PLAY_STRAIGHT; } bool Looping() const { return mPlayMode == PLAY_LOOPED; } - bool Scrubbing() const { return mPlayMode == PLAY_SCRUB; } + bool Scrubbing() const { return mPlayMode == PLAY_SCRUB || mPlayMode == PLAY_KEYBOARD_SCRUB; } bool PlayingAtSpeed() const { return mPlayMode == PLAY_AT_SPEED; } bool Interactive() const { return Scrubbing() || PlayingAtSpeed(); } diff --git a/src/Mix.cpp b/src/Mix.cpp index ac1a73faf..4c91a3411 100644 --- a/src/Mix.cpp +++ b/src/Mix.cpp @@ -762,6 +762,12 @@ void Mixer::SetTimesAndSpeed(double t0, double t1, double speed) Reposition(t0); } +void Mixer::SetSpeed(double speed) +{ + wxASSERT(std::isfinite(speed)); + mSpeed = fabs(speed); +} + MixerSpec::MixerSpec( unsigned numTracks, unsigned maxNumChannels ) { mNumTracks = mNumChannels = numTracks; diff --git a/src/Mix.h b/src/Mix.h index 39a66bb76..7a5753610 100644 --- a/src/Mix.h +++ b/src/Mix.h @@ -134,6 +134,7 @@ class AUDACITY_DLL_API Mixer { // Used in scrubbing. void SetTimesAndSpeed(double t0, double t1, double speed); + void SetSpeed(double speed); /// Current time in seconds (unwarped, i.e. always between startTime and stopTime) /// This value is not accurate, it's useful for progress bars and indicators, but nothing else. diff --git a/src/tracks/ui/Scrubbing.cpp b/src/tracks/ui/Scrubbing.cpp index 2b0f96a33..0001bcdd2 100644 --- a/src/tracks/ui/Scrubbing.cpp +++ b/src/tracks/ui/Scrubbing.cpp @@ -406,6 +406,7 @@ bool Scrubber::MaybeStartScrubbing(wxCoord xx) options.envelope = nullptr; mOptions.delay = (ScrubPollInterval_ms / 1000.0); mOptions.isPlayingAtSpeed = false; + mOptions.isKeyboardScrubbing = false; mOptions.minSpeed = 0.0; #ifdef USE_TRANSCRIPTION_TOOLBAR if (!mAlwaysSeeking) { @@ -530,6 +531,7 @@ bool Scrubber::StartSpeedPlay(double speed, double time0, double time1) mOptions.bySpeed = true; mOptions.adjustStart = false; mOptions.isPlayingAtSpeed = true; + mOptions.isKeyboardScrubbing = false; const bool backwards = time1 < time0; #ifdef EXPERIMENTAL_SCRUBBING_SCROLL_WHEEL @@ -606,6 +608,7 @@ bool Scrubber::StartKeyboardScrubbing(double time0, bool backwards) mOptions.bySpeed = true; mOptions.adjustStart = false; mOptions.isPlayingAtSpeed = false; + mOptions.isKeyboardScrubbing = true; // Must start the thread and poller first or else PlayPlayRegion // will insert some silence