commit 74bc43245c32f0349182dea50406446df875ad7a
parent 4ebd88606b72b89cae5ed2b7d37b96a84635ff3d
Author: Karl Tomlinson <karlt+@karlt.net>
Date: Wed, 8 Oct 2025 07:55:12 +0000
Bug 1992922 Remove GraphDriver::mIterationEnd and synchronize graph rendering across driver switches r=padenot
mIterationEnd was a remnant from when the graph calculated state after each
rendering interval.
Since https://hg.mozilla.org/mozilla-central/rev/51ab07894b49, each iteration
first calculates state and then immediately renders up to state time.
mIterationEnd was still used to coordinate timing when switching drivers, but
the purpose and effect of the logic was unclear since 51ab07894b49. This is
replaced with mIterationTimeStamp and new logic aiming to render at consistent
intervals/times across driver switches.
Underruns don't happen within AudioCallbackDriver::DataCallback() because it
always provides the number of samples requested, which is the clock.
Differential Revision: https://phabricator.services.mozilla.com/D267740
Diffstat:
6 files changed, 119 insertions(+), 86 deletions(-)
diff --git a/dom/media/AudioBufferUtils.h b/dom/media/AudioBufferUtils.h
@@ -83,9 +83,16 @@ class AudioCallbackBufferWrapper {
* Write some frames to the internal buffer. Free space in the buffer should
* be checked prior to calling these.
*/
+ void WriteSilence(const uint32_t aFrames) {
+ MOZ_ASSERT(aFrames <= Available(),
+ "Writing more than we can in the audio buffer.");
+
+ std::fill_n(mBuffer + mSampleWriteOffset, aFrames, static_cast<T>(0));
+ mSampleWriteOffset += FramesToSamples(mChannels, aFrames);
+ }
void WriteFrames(T* aBuffer, uint32_t aFrames) {
MOZ_ASSERT(aFrames <= Available(),
- "Writing more that we can in the audio buffer.");
+ "Writing more than we can in the audio buffer.");
PodCopy(mBuffer + mSampleWriteOffset, aBuffer,
FramesToSamples(mChannels, aFrames));
@@ -93,7 +100,7 @@ class AudioCallbackBufferWrapper {
}
void WriteFrames(const AudioChunk& aChunk, uint32_t aFrames) {
MOZ_ASSERT(aFrames <= Available(),
- "Writing more that we can in the audio buffer.");
+ "Writing more than we can in the audio buffer.");
InterleaveAndConvertBuffer(aChunk.ChannelData<T>().Elements(), aFrames,
aChunk.mVolume, aChunk.ChannelCount(),
diff --git a/dom/media/GraphDriver.cpp b/dom/media/GraphDriver.cpp
@@ -54,13 +54,13 @@ void GraphDriver::SetStreamName(const nsACString& aStreamName) {
}
void GraphDriver::SetState(const nsACString& aStreamName,
- GraphTime aIterationEnd,
- GraphTime aStateComputedTime) {
+ GraphTime aStateComputedTime,
+ TimeStamp aIterationTimeStamp) {
MOZ_ASSERT(InIteration() || !ThreadRunning());
mStreamName = aStreamName;
- mIterationEnd = aIterationEnd;
mStateComputedTime = aStateComputedTime;
+ mTargetIterationTimeStamp = aIterationTimeStamp;
}
#ifdef DEBUG
@@ -179,8 +179,7 @@ SystemClockDriver::SystemClockDriver(GraphInterface* aGraphInterface,
GraphDriver* aPreviousDriver,
uint32_t aSampleRate)
: ThreadedDriver(aGraphInterface, aPreviousDriver, aSampleRate),
- mInitialTimeStamp(TimeStamp::Now()),
- mTargetIterationTimeStamp(TimeStamp::Now()) {}
+ mInitialTimeStamp(TimeStamp::Now()) {}
SystemClockDriver::~SystemClockDriver() = default;
@@ -190,24 +189,13 @@ void ThreadedDriver::RunThread() {
WaitForNextIteration();
MediaTime interval = GetIntervalForIteration();
- auto iterationStart = mIterationEnd;
- mIterationEnd += interval;
- MOZ_ASSERT(iterationStart <= mIterationEnd);
-
- if (mStateComputedTime < mIterationEnd) {
- LOG(LogLevel::Warning, ("%p: Global underrun detected", Graph()));
- mIterationEnd = mStateComputedTime;
- }
-
GraphTime nextStateComputedTime = mStateComputedTime + interval;
LOG(LogLevel::Verbose,
- ("%p: interval[%ld; %ld] state[%ld; %ld]", Graph(),
- (long)iterationStart, (long)mIterationEnd, (long)mStateComputedTime,
+ ("%p: interval[%ld; %ld]", Graph(), (long)mStateComputedTime,
(long)nextStateComputedTime));
mStateComputedTime = nextStateComputedTime;
- IterationResult result =
- Graph()->OneIteration(mStateComputedTime, mIterationEnd, nullptr);
+ IterationResult result = Graph()->OneIteration(mStateComputedTime, nullptr);
if (result.IsStop()) {
// Signal that we're done stopping.
@@ -217,7 +205,8 @@ void ThreadedDriver::RunThread() {
if (GraphDriver* nextDriver = result.NextDriver()) {
LOG(LogLevel::Debug, ("%p: Switching to AudioCallbackDriver", Graph()));
result.Switched();
- nextDriver->SetState(mStreamName, mIterationEnd, mStateComputedTime);
+ nextDriver->SetState(mStreamName, mStateComputedTime,
+ mTargetIterationTimeStamp);
nextDriver->Start();
break;
}
@@ -249,7 +238,13 @@ TimeDuration SystemClockDriver::NextIterationWaitDuration() {
MOZ_ASSERT(mThread);
MOZ_ASSERT(OnThread());
TimeStamp now = TimeStamp::Now();
- mTargetIterationTimeStamp += IterationDuration();
+ if (mTargetIterationTimeStamp.IsNull()) {
+ // No previous driver with which to synchronize rendering.
+ // Start rendering now.
+ mTargetIterationTimeStamp = now;
+ } else {
+ mTargetIterationTimeStamp += IterationDuration();
+ }
TimeDuration timeout = mTargetIterationTimeStamp - now;
if (timeout < TimeDuration::FromMilliseconds(-MEDIA_GRAPH_TARGET_PERIOD_MS)) {
// Rendering has fallen so far behind that the entire next rendering
@@ -291,13 +286,14 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
public:
FallbackWrapper(RefPtr<GraphInterface> aGraph,
RefPtr<AudioCallbackDriver> aOwner, uint32_t aSampleRate,
- const nsACString& aStreamName, GraphTime aIterationEnd,
- GraphTime aStateComputedTime)
+ const nsACString& aStreamName, GraphTime aStateComputedTime,
+ TimeStamp aIterationTimeStamp)
: mGraph(std::move(aGraph)),
mOwner(std::move(aOwner)),
mFallbackDriver(
MakeRefPtr<SystemClockDriver>(this, nullptr, aSampleRate)) {
- mFallbackDriver->SetState(aStreamName, aIterationEnd, aStateComputedTime);
+ mFallbackDriver->SetState(aStreamName, aStateComputedTime,
+ aIterationTimeStamp);
}
NS_DECL_THREADSAFE_ISUPPORTS
@@ -341,7 +337,6 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
}
#endif
IterationResult OneIteration(GraphTime aStateComputedEnd,
- GraphTime aIterationEnd,
MixerCallbackReceiver* aMixerReceiver) override {
MOZ_ASSERT(!aMixerReceiver);
@@ -350,7 +345,7 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
#endif
IterationResult result =
- mGraph->OneIteration(aStateComputedEnd, aIterationEnd, aMixerReceiver);
+ mGraph->OneIteration(aStateComputedEnd, aMixerReceiver);
AudioStreamState audioState = mOwner->mAudioStreamState;
@@ -386,13 +381,14 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
IterationResult stopFallback =
IterationResult::CreateStop(NS_NewRunnableFunction(
"AudioCallbackDriver::FallbackDriverStopped",
- [self = RefPtr<FallbackWrapper>(this), this, aIterationEnd,
- aStateComputedEnd, result = std::move(result)]() mutable {
+ [self = RefPtr<FallbackWrapper>(this), this, aStateComputedEnd,
+ result = std::move(result)]() mutable {
FallbackDriverState fallbackState =
result.IsStillProcessing() ? FallbackDriverState::None
: FallbackDriverState::Stopped;
- mOwner->FallbackDriverStopped(aIterationEnd, aStateComputedEnd,
- fallbackState);
+ mOwner->FallbackDriverStopped(
+ aStateComputedEnd, mFallbackDriver->IterationTimeStamp(),
+ fallbackState);
if (fallbackState == FallbackDriverState::Stopped) {
#ifdef DEBUG
@@ -405,8 +401,8 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
("%p: Switching from fallback to other driver.",
mOwner.get()));
result.Switched();
- nextDriver->SetState(mOwner->mStreamName, aIterationEnd,
- aStateComputedEnd);
+ nextDriver->SetState(mOwner->mStreamName, aStateComputedEnd,
+ mFallbackDriver->IterationTimeStamp());
nextDriver->Start();
} else if (result.IsStop()) {
LOG(LogLevel::Debug,
@@ -859,6 +855,8 @@ bool AudioCallbackDriver::CheckThreadIdChanged() {
long AudioCallbackDriver::DataCallback(const AudioDataValue* aInputBuffer,
AudioDataValue* aOutputBuffer,
long aFrames) {
+ TimeStamp iterationStartTimeStamp = TimeStamp::Now();
+
if (!mSandboxed && CheckThreadIdChanged()) {
CallbackThreadRegistry::Get()->Register(mAudioThreadId,
"NativeAudioCallback");
@@ -867,6 +865,7 @@ long AudioCallbackDriver::DataCallback(const AudioDataValue* aInputBuffer,
if (mAudioStreamState.compareExchange(AudioStreamState::Starting,
AudioStreamState::Running)) {
MOZ_ASSERT(mScratchBuffer.IsEmpty());
+ mFirstCallbackIteration = true;
LOG(LogLevel::Verbose, ("%p: AudioCallbackDriver %p First audio callback "
"close the Fallback driver",
Graph(), this));
@@ -918,49 +917,56 @@ long AudioCallbackDriver::DataCallback(const AudioDataValue* aInputBuffer,
mBuffer.SetBuffer(aOutputBuffer, aFrames);
// fill part or all with leftover data from last iteration (since we
// align to Audio blocks)
- uint32_t alreadyBuffered = mScratchBuffer.Empty(mBuffer);
+ uint32_t prefilledFrameCount = mScratchBuffer.Empty(mBuffer);
+
+ if (mFirstCallbackIteration && !mTargetIterationTimeStamp.IsNull()) {
+ MediaTime renderingTime =
+ MediaTrackGraphImpl::RoundUpToEndOfAudioBlock(SecondsToMediaTime(
+ (iterationStartTimeStamp - mTargetIterationTimeStamp).ToSeconds()));
+ // There is no previous iteration from which to carry over mScratchBuffer.
+ MOZ_ASSERT(prefilledFrameCount == 0);
+ if (renderingTime < aFrames) {
+ // The audio data callback has occurred soon after the previous driver's
+ // rendering time. Synchronize the rendering times of graph frames
+ // under the audio callback with the rendering times under the previous
+ // driver by padding the start of the provided buffer with silence.
+ prefilledFrameCount = AssertedCast<uint32_t>(aFrames - renderingTime);
+ mBuffer.WriteSilence(prefilledFrameCount);
+ }
+ }
+ mFirstCallbackIteration = false;
- // State computed time is decided by the audio callback's buffer length. We
- // compute the iteration start and end from there, trying to keep the amount
- // of buffering in the graph constant.
+ // State computed time is decided by the audio callback's buffer length.
+ GraphTime bufferEndGraphTime = mStateComputedTime + mBuffer.Available();
GraphTime nextStateComputedTime =
- MediaTrackGraphImpl::RoundUpToEndOfAudioBlock(mStateComputedTime +
- mBuffer.Available());
- auto iterationStart = mIterationEnd;
- // inGraph is the number of audio frames there is between the state time and
- // the current time, i.e. the maximum theoretical length of the interval we
- // could use as [iterationStart; mIterationEnd].
- GraphTime inGraph = mStateComputedTime - iterationStart;
- // We want the interval [iterationStart; mIterationEnd] to be before the
- // interval [mStateComputedTime; nextStateComputedTime]. We also want
- // the distance between these intervals to be roughly equivalent each time, to
- // ensure there is no clock drift between current time and state time. Since
- // we can't act on the state time because we have to fill the audio buffer, we
- // reclock the current time against the state time, here.
- mIterationEnd = iterationStart + 0.8 * inGraph;
-
+ MediaTrackGraphImpl::RoundUpToEndOfAudioBlock(bufferEndGraphTime);
LOG(LogLevel::Verbose,
- ("%p: interval[%ld; %ld] state[%ld; %ld] (frames: %ld) (durationMS: %u) "
+ ("%p: interval[%ld; %ld] (frames: %ld) (durationMS: %u) "
"(duration ticks: %ld)",
- Graph(), (long)iterationStart, (long)mIterationEnd,
- (long)mStateComputedTime, (long)nextStateComputedTime, (long)aFrames,
- (uint32_t)durationMS,
+ Graph(), (long)mStateComputedTime, (long)nextStateComputedTime,
+ (long)aFrames, (uint32_t)durationMS,
(long)(nextStateComputedTime - mStateComputedTime)));
- if (mStateComputedTime < mIterationEnd) {
- LOG(LogLevel::Error, ("%p: Media graph global underrun detected", Graph()));
- MOZ_ASSERT_UNREACHABLE("We should not underrun in full duplex");
- mIterationEnd = mStateComputedTime;
- }
+ // mTargetIterationTimeStamp is used to synchronize the timing of rendering
+ // when switching to a different driver. iterationStartTimeStamp, which
+ // corresponds to bufferEndGraphTime, is adjusted for the extra rendering by
+ // the graph due to rounding to block boundaries, so that
+ // mTargetIterationTimeStamp corresponds to nextStateComputedTime.
+ // This is calculated for all iterations, not just when the IterationResult
+ // indicates a switch, because DeviceChangedCallback() might start a
+ // fallback driver for the next iteration.
+ mTargetIterationTimeStamp =
+ iterationStartTimeStamp +
+ MediaTimeToTimeDuration(nextStateComputedTime - bufferEndGraphTime);
// Process mic data if any/needed
if (aInputBuffer && mInputChannelCount > 0) {
Graph()->NotifyInputData(aInputBuffer, static_cast<size_t>(aFrames),
- mSampleRate, mInputChannelCount, alreadyBuffered);
+ mSampleRate, mInputChannelCount,
+ prefilledFrameCount);
}
- IterationResult result =
- Graph()->OneIteration(nextStateComputedTime, mIterationEnd, this);
+ IterationResult result = Graph()->OneIteration(nextStateComputedTime, this);
mStateComputedTime = nextStateComputedTime;
@@ -1018,7 +1024,8 @@ long AudioCallbackDriver::DataCallback(const AudioDataValue* aInputBuffer,
}
result.Switched();
mAudioStreamState = AudioStreamState::Stopping;
- nextDriver->SetState(mStreamName, mIterationEnd, mStateComputedTime);
+ nextDriver->SetState(mStreamName, mStateComputedTime,
+ iterationStartTimeStamp);
nextDriver->Start();
if (!mSandboxed) {
CallbackThreadRegistry::Get()->Unregister(mAudioThreadId);
@@ -1307,9 +1314,9 @@ void AudioCallbackDriver::FallbackToSystemClockDriver() {
mNextReInitBackoffStep =
TimeDuration::FromMilliseconds(AUDIO_INITIAL_FALLBACK_BACKOFF_STEP_MS);
mNextReInitAttempt = TimeStamp::Now() + mNextReInitBackoffStep;
- auto fallback =
- MakeRefPtr<FallbackWrapper>(Graph(), this, mSampleRate, mStreamName,
- mIterationEnd, mStateComputedTime);
+ auto fallback = MakeRefPtr<FallbackWrapper>(Graph(), this, mSampleRate,
+ mStreamName, mStateComputedTime,
+ mTargetIterationTimeStamp);
{
auto driver = mFallback.Lock();
MOZ_RELEASE_ASSERT(!driver.ref());
@@ -1318,14 +1325,14 @@ void AudioCallbackDriver::FallbackToSystemClockDriver() {
fallback->Start();
}
-void AudioCallbackDriver::FallbackDriverStopped(GraphTime aIterationEnd,
- GraphTime aStateComputedTime,
+void AudioCallbackDriver::FallbackDriverStopped(GraphTime aStateComputedTime,
+ TimeStamp aIterationTimeStamp,
FallbackDriverState aState) {
LOG(LogLevel::Debug,
("%p: AudioCallbackDriver %p Fallback driver has stopped.", Graph(),
this));
- mIterationEnd = aIterationEnd;
mStateComputedTime = aStateComputedTime;
+ mTargetIterationTimeStamp = aIterationTimeStamp;
mNextReInitAttempt = TimeStamp();
mNextReInitBackoffStep = TimeDuration();
{
diff --git a/dom/media/GraphDriver.h b/dom/media/GraphDriver.h
@@ -175,8 +175,7 @@ struct GraphInterface : public nsISupports {
/* Called by GraphDriver to iterate the graph. Mixed audio output from the
* graph is passed into aMixerReceiver, if it is non-null. */
virtual IterationResult OneIteration(
- GraphTime aStateComputedEnd, GraphTime aIterationEnd,
- MixerCallbackReceiver* aMixerReceiver) = 0;
+ GraphTime aStateComputedEnd, MixerCallbackReceiver* aMixerReceiver) = 0;
#ifdef DEBUG
/* True if we're on aDriver's thread, or if we're on mGraphRunner's thread
* and mGraphRunner is currently run by aDriver. */
@@ -296,9 +295,18 @@ class GraphDriver {
/**
* Set the state of the driver so it can start at the right point in time,
* after switching from another driver.
+ *
+ * aIterationTimeStamp is the system clock time from when rendering began in
+ * the most recent iteration. An audio callback is assumed invoked soon
+ * after the end of the provided portion of the platform audio buffer
+ * becomes available for reading or writing, and so provides a clock edge
+ * such that aCurrentTimeStamp advances consistently with the output time of
+ * the end of the set of frames rendered in each iteration. Actual audio
+ * output time of the last data written would be at least a platform buffer
+ * length after a write buffer portion becomes available.
*/
- void SetState(const nsACString& aStreamName, GraphTime aIterationEnd,
- GraphTime aStateComputedTime);
+ void SetState(const nsACString& aStreamName, GraphTime aStateComputedTime,
+ TimeStamp aIterationTimeStamp);
GraphInterface* Graph() const { return mGraphInterface; }
@@ -334,10 +342,19 @@ class GraphDriver {
protected:
// The UTF-8 name for system audio streams. Graph thread.
nsCString mStreamName;
- // Time of the end of this graph iteration.
- GraphTime mIterationEnd = 0;
// Time until which the graph has processed data.
GraphTime mStateComputedTime = 0;
+ // The system clock time when the iteration should or would start if these
+ // start times advance consistently with the number of frames rendered by
+ // the graph in each iteration.
+ // Initially null, if no previous driver exists to provide a reference time
+ // through SetState().
+ // SystemClockDriver advances this before waiting to render the next
+ // iteration.
+ // AudioCallbackDriver sets this to approximately now at the start of each
+ // iteration when !HasFallback().
+ // Unused by OfflineClockDriver.
+ TimeStamp mTargetIterationTimeStamp;
// The GraphInterface this driver is currently iterating.
const RefPtr<GraphInterface> mGraphInterface;
// The sample rate for the graph, and in case of an audio driver, also for the
@@ -473,6 +490,9 @@ class SystemClockDriver final : public ThreadedDriver {
virtual ~SystemClockDriver();
SystemClockDriver* AsSystemClockDriver() override { return this; }
const SystemClockDriver* AsSystemClockDriver() const override { return this; }
+ const TimeStamp& IterationTimeStamp() const {
+ return mTargetIterationTimeStamp;
+ }
protected:
/* Return the TimeDuration to wait before the next rendering iteration. */
@@ -483,10 +503,6 @@ class SystemClockDriver final : public ThreadedDriver {
// Those are only modified (after initialization) on the graph thread. The
// graph thread does not run during the initialization.
TimeStamp mInitialTimeStamp;
- // The system clock time when the next or in-progress iteration should start
- // if the time that rendering happens advances consistently with the frames
- // rendered. Advanced before waiting to render the next iteration.
- TimeStamp mTargetIterationTimeStamp;
};
/**
@@ -678,8 +694,8 @@ class AudioCallbackDriver final : public GraphDriver,
* will be None. If it stopped after the graph told it to stop, or switch,
* aState will be Stopped. Hands over state to the audio driver that may
* iterate the graph after this has been called. */
- void FallbackDriverStopped(GraphTime aIterationEnd,
- GraphTime aStateComputedTime,
+ void FallbackDriverStopped(GraphTime aStateComputedTime,
+ TimeStamp aIterationTimeStamp,
FallbackDriverState aState);
/* Called at the end of the fallback driver's iteration to see whether we
@@ -717,6 +733,10 @@ class AudioCallbackDriver final : public GraphDriver,
*/
const CubebUtils::AudioDeviceID mOutputDeviceID;
const CubebUtils::AudioDeviceID mInputDeviceID;
+ /* Whether the current or a future audio callback will be the first callback
+ * to iterate the graph. Used only from DataCallback().
+ * Initialized on transition from AudioStreamState::Starting to Running. */
+ MOZ_INIT_OUTSIDE_CTOR bool mFirstCallbackIteration;
/* Approximation of the time between two callbacks. This is used to schedule
* video frames. This is in milliseconds. Only even used (after
* inizatialization) on the audio callback thread. */
diff --git a/dom/media/MediaTrackGraph.cpp b/dom/media/MediaTrackGraph.cpp
@@ -1630,7 +1630,6 @@ bool MediaTrackGraphImpl::UpdateMainThreadState() {
}
auto MediaTrackGraphImpl::OneIteration(GraphTime aStateTime,
- GraphTime aIterationEnd,
MixerCallbackReceiver* aMixerReceiver)
-> IterationResult {
if (mGraphRunner) {
diff --git a/dom/media/MediaTrackGraphImpl.h b/dom/media/MediaTrackGraphImpl.h
@@ -295,7 +295,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
* OneIterationImpl is called directly. Mixed audio output from the graph is
* passed into aMixerReceiver, if it is non-null.
*/
- IterationResult OneIteration(GraphTime aStateTime, GraphTime aIterationEnd,
+ IterationResult OneIteration(GraphTime aStateTime,
MixerCallbackReceiver* aMixerReceiver) override;
/**
diff --git a/dom/media/gtest/TestAudioCallbackDriver.cpp b/dom/media/gtest/TestAudioCallbackDriver.cpp
@@ -42,7 +42,7 @@ class MockGraphInterface : public GraphInterface {
#endif
/* OneIteration cannot be mocked because IterationResult is non-memmovable and
* cannot be passed as a parameter, which GMock does internally. */
- IterationResult OneIteration(GraphTime aStateComputedTime, GraphTime,
+ IterationResult OneIteration(GraphTime aStateComputedTime,
MixerCallbackReceiver* aMixerReceiver) {
GraphDriver* driver = mCurrentDriver;
if (aMixerReceiver) {
