Bug 1666116 - Account for fallback in NotifyWhenGraphStarted. r=padenot

Unittests using GoFaster mode may start iterating the audio driver faster before
its fallback has completed handover to the audio callback. This causes the
preSilenceSamples check to spike and the test fails.

Waiting for the fallback to finish handing over fixes this.

To accomodate this, this patch also constifies some methods as needed to check
InIteration() when const. It also waits for audio drivers to start per the above
in TestCrossGraphTrack as that test may otherwise exhibit this bug with bug
1605314 applied.

Differential Revision: https://phabricator.services.mozilla.com/D97413
This commit is contained in:
Andreas Pehrson 2020-11-19 15:50:24 +00:00
Родитель f5294190b5
Коммит ea27597884
8 изменённых файлов: 59 добавлений и 29 удалений

Просмотреть файл

@ -52,7 +52,7 @@ void GraphDriver::SetState(GraphTime aIterationStart, GraphTime aIterationEnd,
}
#ifdef DEBUG
bool GraphDriver::InIteration() {
bool GraphDriver::InIteration() const {
return OnThread() || Graph()->InDriverIteration(this);
}
#endif
@ -399,7 +399,7 @@ class AudioCallbackDriver::FallbackWrapper : public GraphInterface {
MOZ_CRASH("Unexpected DeviceChanged from fallback SystemClockDriver");
}
#ifdef DEBUG
bool InDriverIteration(GraphDriver* aDriver) override {
bool InDriverIteration(const GraphDriver* aDriver) const override {
return !mOwner->ThreadRunning() && mOwner->InIteration();
}
#endif
@ -1231,6 +1231,11 @@ TimeDuration AudioCallbackDriver::AudioOutputLatency() {
mSampleRate);
}
bool AudioCallbackDriver::OnFallback() const {
MOZ_ASSERT(InIteration());
return mFallbackDriverState == FallbackDriverState::Running;
}
void AudioCallbackDriver::FallbackToSystemClockDriver() {
MOZ_ASSERT(!ThreadRunning());
MOZ_ASSERT(mAudioStreamState == AudioStreamState::None ||

Просмотреть файл

@ -199,7 +199,7 @@ struct GraphInterface : public nsISupports {
#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. */
virtual bool InDriverIteration(GraphDriver* aDriver) = 0;
virtual bool InDriverIteration(const GraphDriver* aDriver) const = 0;
#endif
};
@ -298,10 +298,19 @@ class GraphDriver {
void SetPreviousDriver(GraphDriver* aPreviousDriver);
virtual AudioCallbackDriver* AsAudioCallbackDriver() { return nullptr; }
virtual const AudioCallbackDriver* AsAudioCallbackDriver() const {
return nullptr;
}
virtual OfflineClockDriver* AsOfflineClockDriver() { return nullptr; }
virtual const OfflineClockDriver* AsOfflineClockDriver() const {
return nullptr;
}
virtual SystemClockDriver* AsSystemClockDriver() { return nullptr; }
virtual const SystemClockDriver* AsSystemClockDriver() const {
return nullptr;
}
/**
* Set the state of the driver so it can start at the right point in time,
@ -314,12 +323,12 @@ class GraphDriver {
#ifdef DEBUG
// True if the current thread is currently iterating the MTG.
bool InIteration();
bool InIteration() const;
#endif
// True if the current thread is the GraphDriver's thread.
virtual bool OnThread() = 0;
virtual bool OnThread() const = 0;
// GraphDriver's thread has started and the thread is running.
virtual bool ThreadRunning() = 0;
virtual bool ThreadRunning() const = 0;
double MediaTimeToSeconds(GraphTime aTime) const {
NS_ASSERTION(aTime > -TRACK_TIME_MAX && aTime <= TRACK_TIME_MAX,
@ -431,13 +440,13 @@ class ThreadedDriver : public GraphDriver {
friend class MediaTrackGraphInitThreadRunnable;
uint32_t IterationDuration() override { return MEDIA_GRAPH_TARGET_PERIOD_MS; }
nsIThread* Thread() { return mThread; }
nsIThread* Thread() const { return mThread; }
bool OnThread() override {
bool OnThread() const override {
return !mThread || mThread->EventTarget()->IsOnCurrentThread();
}
bool ThreadRunning() override { return mThreadRunning; }
bool ThreadRunning() const override { return mThreadRunning; }
protected:
/* Waits until it's time to process more data. */
@ -474,6 +483,7 @@ class SystemClockDriver : public ThreadedDriver {
GraphDriver* aPreviousDriver, uint32_t aSampleRate);
virtual ~SystemClockDriver();
SystemClockDriver* AsSystemClockDriver() override { return this; }
const SystemClockDriver* AsSystemClockDriver() const override { return this; }
protected:
/* Return the TimeDuration to wait before the next rendering iteration. */
@ -498,6 +508,9 @@ class OfflineClockDriver : public ThreadedDriver {
GraphTime aSlice);
virtual ~OfflineClockDriver();
OfflineClockDriver* AsOfflineClockDriver() override { return this; }
const OfflineClockDriver* AsOfflineClockDriver() const override {
return this;
}
void RunThread() override;
@ -607,6 +620,9 @@ class AudioCallbackDriver : public GraphDriver,
uint32_t aSampleRate) override;
AudioCallbackDriver* AsAudioCallbackDriver() override { return this; }
const AudioCallbackDriver* AsAudioCallbackDriver() const override {
return this;
}
uint32_t OutputChannelCount() { return mOutputChannelCount; }
@ -619,7 +635,7 @@ class AudioCallbackDriver : public GraphDriver,
return AudioInputType::Unknown;
}
std::thread::id ThreadId() { return mAudioThreadIdInCb.load(); }
std::thread::id ThreadId() const { return mAudioThreadIdInCb.load(); }
/* Called when the thread servicing the callback has changed. This can be
* fairly expensive */
@ -628,13 +644,13 @@ class AudioCallbackDriver : public GraphDriver,
* changed. */
bool CheckThreadIdChanged();
bool OnThread() override {
bool OnThread() const override {
return mAudioThreadIdInCb.load() == std::this_thread::get_id();
}
/* Returns true if this audio callback driver has successfully started and not
* yet stopped. If the fallback driver is active, this returns false. */
bool ThreadRunning() override {
bool ThreadRunning() const override {
return mAudioStreamState == AudioStreamState::Running;
}
@ -645,6 +661,9 @@ class AudioCallbackDriver : public GraphDriver,
// Returns the output latency for the current audio output stream.
TimeDuration AudioOutputLatency();
/* Returns true if this driver is currently driven by the fallback driver. */
bool OnFallback() const;
private:
/**
* On certain MacBookPro, the microphone is located near the left speaker.

Просмотреть файл

@ -130,12 +130,12 @@ NS_IMETHODIMP GraphRunner::Run() {
return NS_OK;
}
bool GraphRunner::OnThread() {
bool GraphRunner::OnThread() const {
return mThread->EventTarget()->IsOnCurrentThread();
}
#ifdef DEBUG
bool GraphRunner::InDriverIteration(GraphDriver* aDriver) {
bool GraphRunner::InDriverIteration(const GraphDriver* aDriver) const {
if (!OnThread()) {
return false;
}

Просмотреть файл

@ -46,14 +46,14 @@ class GraphRunner final : public Runnable {
/**
* Returns true if called on mThread.
*/
bool OnThread();
bool OnThread() const;
#ifdef DEBUG
/**
* Returns true if called on mThread, and aDriver was the driver that called
* OneIteration() last.
*/
bool InDriverIteration(GraphDriver* aDriver);
bool InDriverIteration(const GraphDriver* aDriver) const;
#endif
private:

Просмотреть файл

@ -3095,7 +3095,7 @@ AbstractThread* MediaTrackGraph::AbstractMainThread() {
}
#ifdef DEBUG
bool MediaTrackGraphImpl::InDriverIteration(GraphDriver* aDriver) {
bool MediaTrackGraphImpl::InDriverIteration(const GraphDriver* aDriver) const {
return aDriver->OnThread() ||
(mGraphRunner && mGraphRunner->InDriverIteration(aDriver));
}
@ -3447,7 +3447,8 @@ void MediaTrackGraphImpl::NotifyWhenGraphStarted(
// ControlMessage.
MediaTrackGraphImpl* graphImpl = mMediaTrack->GraphImpl();
if (graphImpl->CurrentDriver()->AsAudioCallbackDriver() &&
graphImpl->CurrentDriver()->ThreadRunning()) {
graphImpl->CurrentDriver()->ThreadRunning() &&
!graphImpl->CurrentDriver()->AsAudioCallbackDriver()->OnFallback()) {
// Avoid Resolve's locking on the graph thread by doing it on main.
graphImpl->Dispatch(NS_NewRunnableFunction(
"MediaTrackGraphImpl::NotifyWhenGraphStarted::Resolver",

Просмотреть файл

@ -131,7 +131,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
* True if we're on aDriver's thread, or if we're on mGraphRunner's thread
* and mGraphRunner is currently run by aDriver.
*/
bool InDriverIteration(GraphDriver* aDriver) override;
bool InDriverIteration(const GraphDriver* aDriver) const override;
#endif
/**

Просмотреть файл

@ -65,7 +65,7 @@ class MockGraphInterface : public GraphInterface {
void SetEnsureNextIteration(bool aEnsure) { mEnsureNextIteration = aEnsure; }
#ifdef DEBUG
bool InDriverIteration(GraphDriver* aDriver) override {
bool InDriverIteration(const GraphDriver* aDriver) const override {
return aDriver->OnThread();
}
#endif

Просмотреть файл

@ -213,14 +213,18 @@ TEST(TestAudioTrackGraph, ErrorCallback)
CubebUtils::ForceSetCubebContext(cubeb->AsCubebContext());
MediaTrackGraph* graph = MediaTrackGraph::GetInstance(
MediaTrackGraph::AUDIO_THREAD_DRIVER, /*window*/ nullptr,
MediaTrackGraph::SYSTEM_THREAD_DRIVER, /*window*/ nullptr,
MediaTrackGraph::REQUEST_DEFAULT_SAMPLE_RATE, nullptr);
// Dummy track to make graph rolling. Add it and remove it to remove the
// graph from the global hash table and let it shutdown.
//
// We open an input through this track so that there's something triggering
// EnsureNextIteration on the fallback driver after the callback driver has
// gotten the error.
RefPtr<AudioInputTrack> inputTrack;
RefPtr<AudioInputProcessing> listener;
Unused << WaitFor(Invoke([&] {
auto started = Invoke([&] {
inputTrack = AudioInputTrack::Create(graph);
listener = new AudioInputProcessing(2, PRINCIPAL_HANDLE_NONE);
inputTrack->GraphImpl()->AppendMessage(
@ -228,13 +232,6 @@ TEST(TestAudioTrackGraph, ErrorCallback)
inputTrack->SetInputProcessing(listener);
inputTrack->GraphImpl()->AppendMessage(
MakeUnique<StartInputProcessing>(inputTrack, listener));
return graph->NotifyWhenDeviceStarted(inputTrack);
}));
// We open an input through this track so that there's something triggering
// EnsureNextIteration on the fallback driver after the callback driver has
// gotten the error.
auto started = Invoke([&] {
inputTrack->OpenAudioInput((void*)1, listener);
return graph->NotifyWhenDeviceStarted(inputTrack);
});
@ -663,6 +660,14 @@ void TestCrossGraphPort(uint32_t aInputRate, uint32_t aOutputRate,
return inputStream && partnerStream;
});
Unused << WaitFor(Invoke([&] {
nsTArray<RefPtr<MediaTrackGraph::GraphStartedPromise>> ps(
{primary->NotifyWhenDeviceStarted(inputTrack),
partner->NotifyWhenDeviceStarted(receiver)});
return MediaTrackGraph::GraphStartedPromise::All(
GetCurrentSerialEventTarget(), ps);
}));
partnerStream->SetDriftFactor(aDriftFactor);
// Wait for a second worth of audio data. GoFaster is dispatched through a