diff --git a/USER_MANUAL.md b/USER_MANUAL.md index efd0f124..2f13dcfd 100644 --- a/USER_MANUAL.md +++ b/USER_MANUAL.md @@ -958,6 +958,7 @@ LDPC | Low Density Parity Check Codes - a family of powerful FEC codes * Fix uninitialized value read during waterfall plot render. (PR #1481) * Match std::atomic memory ordering to what each atomic actually synchronises. (PR #1482) * Remove use of mutexes in audio path during callsign decode. (PR #1486) + * Tighten audio thread timings on macOS and Windows. (PR #1494) 2. Build system: * Windows versions are now built with llvm-mingw 20260908 (PR #1489) 3. Other: diff --git a/src/audio/MacAudioDevice.cpp b/src/audio/MacAudioDevice.cpp index 31e81fb8..2efa46c4 100644 --- a/src/audio/MacAudioDevice.cpp +++ b/src/audio/MacAudioDevice.cpp @@ -45,7 +45,6 @@ thread_local int MacAudioDevice::CurrentCoreAudioId_ = 0; // Conversion factors. constexpr static int MS_TO_SEC = 1000; -constexpr static int MS_TO_NSEC = 1000000; // The I/O interval time in seconds. constexpr static int AUDIO_SAMPLE_BLOCK_MSEC = 20; @@ -714,7 +713,7 @@ void MacAudioDevice::setHelperRealTime() // Define constants determining how much time the audio thread can // use in a given time quantum. All times are in milliseconds. - const double kTimeQuantum = 60; // 60ms, 1/2 of a RADEV1 block and confirmed to be sufficient with Instruments analysis. + const double kTimeQuantum = 60; // 60ms, calculated by AUDIO_SAMPLE_BLOCK_WIRELESS_MSEC / kGuaranteedAudioDutyCycle + a bit extra. // Time guaranteed each quantum. const double kAudioTimeNeeded = kGuaranteedAudioDutyCycle * kTimeQuantum; @@ -911,27 +910,32 @@ void MacAudioDevice::startRealTimeWork() void MacAudioDevice::stopRealTimeWork(bool fastMode) { - int64_t timeToWaitMilliseconds = ((1000 * chosenFrameSize_) / sampleRate_) >> (fastMode ? 1 : 0); + int64_t nominalUs = ((1000000LL * chosenFrameSize_) / sampleRate_) >> (fastMode ? 1 : 0); - // If last cycle's total (processing + wait, measured from - // startRealTimeWork() above) ran longer than its nominal period, shave - // that overrun off this cycle's wait -- otherwise every cycle where - // processing takes nonzero time makes the loop's average period longer - // than intended, drifting later relative to real time instead of - // self-correcting. Matches WASAPIAudioDevice/PulseAudioDevice, which - // had this already; this device previously didn't. - timeToWaitMilliseconds -= extraTimeMs_; - if (timeToWaitMilliseconds <= 0) + // Compensate for how much of the period THIS cycle's own processing + // already used, measured directly against startTime_ (set by + // startRealTimeWork() right before processing began) rather than a debt + // figure copied from the *previous* cycle. The previous approach lagged + // by one cycle: it corrected this wait for last cycle's overrun instead + // of this cycle's own, which overcorrects/undercorrects whenever + // processing time varies cycle to cycle instead of holding steady. + // waitOvershootUs_ separately tracks only the wait itself running long + // (the one thing that genuinely can't be known until after it happens), + // so a systematic scheduling overshoot still can't accumulate into + // drift. Matches WASAPIAudioDevice's stopRealTimeWork() fix (36db96e5). + auto elapsedUs = std::chrono::duration_cast(std::chrono::steady_clock::now() - startTime_).count(); + int64_t waitUs = nominalUs - elapsedUs - waitOvershootUs_; + if (waitUs <= 0) { - extraTimeMs_ = 0; + waitOvershootUs_ = 0; return; } - dispatch_semaphore_wait(sem_, dispatch_time(DISPATCH_TIME_NOW, MS_TO_NSEC * timeToWaitMilliseconds)); + auto waitStartTime = std::chrono::steady_clock::now(); + dispatch_semaphore_wait(sem_, dispatch_time(DISPATCH_TIME_NOW, 1000 * waitUs)); - auto endTime = std::chrono::steady_clock::now(); - auto durationUs = std::chrono::duration_cast(endTime - startTime_).count() - (1000 * timeToWaitMilliseconds); - extraTimeMs_ = std::max((int64_t)0, (durationUs + 500) / 1000); // round to nearest ms, floor at 0. + auto actualWaitUs = std::chrono::duration_cast(std::chrono::steady_clock::now() - waitStartTime).count(); + waitOvershootUs_ = std::max((int64_t)0, actualWaitUs - waitUs); // cap at >= 0; an early (semaphore) wake isn't overshoot. } void MacAudioDevice::clearHelperRealTime() diff --git a/src/audio/MacAudioDevice.h b/src/audio/MacAudioDevice.h index 2372c5e8..955e9a9b 100644 --- a/src/audio/MacAudioDevice.h +++ b/src/audio/MacAudioDevice.h @@ -88,12 +88,12 @@ private: int chosenFrameSize_; std::atomic numRealTimeWorkers_; - // For handling additional wakeup time after semaphore timeout, matching - // WASAPIAudioDevice/PulseAudioDevice: if last cycle's total (processing - // + wait) ran long, shave that overrun off this cycle's wait so the - // average loop period stays locked to the nominal rate instead of - // drifting later every cycle that processing takes nonzero time. - int64_t extraTimeMs_ = 0; + // Tracks only how long the *wait itself* overshot its requested duration + // last cycle (microseconds), separate from processing time -- which + // stopRealTimeWork() measures directly each cycle against startTime_ + // rather than inferring it from this. See stopRealTimeWork() for why; + // matches WASAPIAudioDevice's equivalent fields/fix (36db96e5). + int64_t waitOvershootUs_ = 0; std::chrono::time_point startTime_; void stopImpl_(); diff --git a/src/audio/WASAPIAudioDevice.cpp b/src/audio/WASAPIAudioDevice.cpp index 88d7cba6..76a45e78 100644 --- a/src/audio/WASAPIAudioDevice.cpp +++ b/src/audio/WASAPIAudioDevice.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -332,7 +333,7 @@ void WASAPIAudioDevice::start() // Allocate temporary buffer tmpBuf_ = new short[sampleRate_]; assert(tmpBuf_ != nullptr); - memset(tmpBuf_, 0, bufferFrameCount_ * numChannels_ * sizeof(short)); + memset(tmpBuf_, 0, sizeof(short) * sampleRate_); if (direction_ == IAudioEngine::AUDIO_ENGINE_OUT) { @@ -575,6 +576,21 @@ void WASAPIAudioDevice::setHelperRealTime() if (HelperTask_ == nullptr) { log_warn("Could not increase thread priority"); + return; + } + + // AvSetMmThreadCharacteristics() alone only enrolls the thread in the + // "Pro Audio" MMCSS class at that class's default (Normal) priority + // band. This thread's wait in stopRealTimeWork() is on the critical + // path for audio timing (it's what TxRxThread's wait/TX/RX stats + // measure), so bump it to the top of the band to cut down on how long + // it sits ready-but-not-running behind other MMCSS-scheduled threads + // after the semaphore/timer wakes it -- that scheduling delay is what + // shows up as wait jitter (stdev/max) rather than the wait target + // itself being wrong. + if (!AvSetMmThreadPriority(HelperTask_, AVRT_PRIORITY_CRITICAL)) + { + log_warn("Could not raise MMCSS thread priority to critical (err = %lu)", GetLastError()); } } diff --git a/src/audio/WASAPIAudioDevice.h b/src/audio/WASAPIAudioDevice.h index ea281665..fa8ec0ae 100644 --- a/src/audio/WASAPIAudioDevice.h +++ b/src/audio/WASAPIAudioDevice.h @@ -129,7 +129,7 @@ void WASAPIAudioDevice::copyFloatToShort_(T* source, int numFrames) { for (int innerIndex = 0; innerIndex < numChannels_; innerIndex++) { - tmpBuf_[index * numChannels_ + innerIndex] = source[index * numChannels_ + innerIndex] * std::numeric_limits::max(); + tmpBuf_[index * numChannels_ + innerIndex] = source[index * numChannels_ + innerIndex] * (std::numeric_limits::max()); } } } @@ -141,7 +141,7 @@ void WASAPIAudioDevice::copyShortToFloat_(T* dest, int numFrames) { for (int innerIndex = 0; innerIndex < numChannels_; innerIndex++) { - dest[index * numChannels_ + innerIndex] = (T)tmpBuf_[index * numChannels_ + innerIndex] / std::numeric_limits::max(); + dest[index * numChannels_ + innerIndex] = (T)tmpBuf_[index * numChannels_ + innerIndex] / (std::numeric_limits::max()); } } }