From 582e574ab85e73169340df6d8fb606ac4eeb3f63 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 27 Aug 2017 03:26:44 -0400 Subject: [PATCH] A not-harmful change to MIDI timing for Windows and Mac... ... and hoping it is positively helpful for Linux. In AudioIO::MidiTime(), compute one of the terms by different means. Use PaStreamInfo::outputLatency. Do not use the difference of PaStreamCallbackTimeInfo::outputBufferDacTime and PaStreamCallbackTimeInfo::currentTime. Which debugging shows is very nearly the same value for Windows and Mac. But we suspect the PaStreamCallbackTimeInfo fields are not correctly reported on Linux. --- src/AudioIO.cpp | 42 +++++++++++++++++++++++++++++++++--------- src/AudioIO.h | 17 ++++++++++++----- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/AudioIO.cpp b/src/AudioIO.cpp index 13b4d113f..beb99e285 100644 --- a/src/AudioIO.cpp +++ b/src/AudioIO.cpp @@ -315,6 +315,13 @@ wxArrayLong AudioIO::mCachedSampleRates; double AudioIO::mCachedBestRateIn = 0.0; double AudioIO::mCachedBestRateOut; +enum { + // This is the least positive latency we can + // specify to Pm_OpenOutput, 1 ms, which prevents immediate + // scheduling of events: + MIDI_MINIMAL_LATENCY_MS = 1 +}; + #ifdef EXPERIMENTAL_SCRUBBING_SUPPORT #include "tracks/ui/Scrubbing.h" @@ -1525,6 +1532,7 @@ bool AudioIO::StartPortAudioStream(double sampleRate, int userData = 24; int* lpUserData = (captureFormat_saved == int24Sample) ? &userData : NULL; + mMidiTimeCorrection = 0; mLastPaError = Pa_OpenStream( &mPortStreamV19, useCapture ? &captureParameters : NULL, usePlayback ? &playbackParameters : NULL, @@ -1538,6 +1546,10 @@ bool AudioIO::StartPortAudioStream(double sampleRate, Px_SetInputVolume(mPortMixer, oldRecordVolume); #endif if (mPortStreamV19 != NULL && mLastPaError == paNoError) { + auto info = Pa_GetStreamInfo(mPortStreamV19); + if (info) + mMidiTimeCorrection = + info->outputLatency - (MIDI_MINIMAL_LATENCY_MS / 1000.0); #ifdef __WXMAC__ if (mPortMixer) { if (Px_SupportsPlaythrough(mPortMixer)) { @@ -2110,7 +2122,6 @@ bool AudioIO::StartPortMidiStream() if (nTracks == 0) return false; - mMidiLatency = 1; // arbitrary, but small //printf("StartPortMidiStream: mT0 %g mTime %g\n", // gAudioIO->mT0, gAudioIO->mTime); @@ -2141,7 +2152,7 @@ bool AudioIO::StartPortMidiStream() 0, &::MidiTime, NULL, - mMidiLatency); + MIDI_MINIMAL_LATENCY_MS); if (mLastPmError == pmNoError) { mMidiStreamActive = true; mMidiPaused = false; @@ -3829,7 +3840,7 @@ void AudioIO::OutputEvent() // 0.0005 is for rounding double time = eventTime + 0.0005 - - ((mMidiLatency + mSynthLatency) * 0.001); + (mSynthLatency * 0.001); time += 1; // MidiTime() has a 1s offset // state changes have to go out without delay because the @@ -4034,17 +4045,25 @@ PmTimestamp AudioIO::MidiTime() // note: the extra 0.0005 is for rounding. Round down by casting to // unsigned long, then convert to PmTimeStamp (currently signed) - // PRL: Bug1714 happened because PaUtil_GetTime() and - // mAudioCallbackOutputDacTime could be very widely different, causing - // integer overlows. - // Portaudio documentation says the origin for - // the times passed to audacityAudioCallback is unspecified. // See long comments at the top of the file for the explanation of this // calculation; we must use PaUtil_GetTime() here and also in the audio // callback, to change the origin of times from portaudio, so the diffence of // now and then is small, as the long comment assumes. + + // PRL: the time correction is really Midi latency achieved by different + // means than specifiying it to Pm_OpenStream. The use of the accumulated + // sample count generated by the audio callback (in AudioTime()) might also + // have the virtue of keeping the Midi output synched with audio, even though + // pmlinuxalsa.c does not implement any synchronization of its own. + // We are now using mMidiTimeCorrection, computed once after opening the + // portaudio stream, rather than the difference of dac and current + // times reported to the audio callback; because the answer is very nearly + // the same on Windows and macOs, doing no harm, whereas on Linux with ALSA, + // the dac and current times were not reliable, and that caused irregular + // timing of Midi playback because latency was effectively zero. auto clockChange = PaUtil_GetTime() - mAudioCallbackClockTime; - auto offset = mAudioCallbackOutputDacTime - mAudioCallbackOutputCurrentTime; + // auto offset = mAudioCallbackOutputDacTime - mAudioCallbackOutputCurrentTime; + auto offset = mMidiTimeCorrection; return (PmTimestamp) ((unsigned long) (1000 * ( AudioTime() + 1.0005 - mAudioFramesPerBuffer / mRate + @@ -4283,8 +4302,13 @@ int audacityAudioCallback(const void *inputBuffer, void *outputBuffer, #ifdef EXPERIMENTAL_MIDI_OUT /* GSW: Save timeInfo in case MidiPlayback needs it */ gAudioIO->mAudioCallbackClockTime = PaUtil_GetTime(); + + // PRL: We no longer trust these numbers on Linux / ALSA: + /* gAudioIO->mAudioCallbackOutputDacTime = timeInfo->outputBufferDacTime; gAudioIO->mAudioCallbackOutputCurrentTime = timeInfo->currentTime; + */ + // printf("in callback, mAudioCallbackOutputDacTime %g\n", gAudioIO->mAudioCallbackOutputDacTime); //DBG gAudioIO->mAudioFramesPerBuffer = framesPerBuffer; if(gAudioIO->IsPaused()) diff --git a/src/AudioIO.h b/src/AudioIO.h index 29bf74b86..b0971c08e 100644 --- a/src/AudioIO.h +++ b/src/AudioIO.h @@ -528,20 +528,27 @@ private: // MIDI_PLAYBACK: PmStream *mMidiStream; PmError mLastPmError; - /// Latency value for PortMidi - long mMidiLatency; + PaTime mMidiTimeCorrection; // seconds /// Latency of MIDI synthesizer - long mSynthLatency; + long mSynthLatency; // ms // These fields are used to synchronize MIDI with audio: /// PortAudio's clock time volatile double mAudioCallbackClockTime; - /// PortAudio's currentTime -- its origin is unspecified! So that's why - /// we also record the above + + /// PRL: no longer using the next two because they are not reliably + /// reported to the audio callback on Linux. + /// Compute the approximate difference of them by other means now: + /// use PaStreamInfo::outputLatency. + + /* + /// PortAudio's currentTime -- its origin is unspecified! volatile double mAudioCallbackOutputCurrentTime; /// PortAudio's outTime volatile double mAudioCallbackOutputDacTime; + */ + /// Number of frames output, including pauses volatile long mNumFrames; /// How many frames of zeros were output due to pauses?