Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Commit 1e5f259

Browse files
author
[email protected]
committed
[GPUProcess] Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination
https://bugs.webkit.org/show_bug.cgi?id=218251 Reviewed by Geoffrey Garen. Source/WebCore: DefaultAudioDestinationNode::resume() / suspend() were already asynchronous operations. However, they expected AudioDestination::start() / stop() to finish synchronously and would simply call their completion handler asynchronously. Instead, we now make AudioDestination::start() / stop() asynchronous as well. This allows us to use asynchronous IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination. As a result of this change, I had to make AudioDestinationNode::startRendering() asynchronous as well since it uses AudioDestination::start() internally. As an improvement, the completion handler to AudioDestinationNode's startRendering() / resume() / suspend() is now provided with an exception in cases where they fail. This allows the call sites to properly deal with such errors instead of assuming things were successsful. No new tests, no Web-facing beahvior change. * Modules/webaudio/AudioContext.cpp: (WebCore::AudioContext::suspendRendering): (WebCore::AudioContext::resumeRendering): (WebCore::AudioContext::startRendering): (WebCore::AudioContext::mayResumePlayback): (WebCore::AudioContext::suspendPlayback): * Modules/webaudio/AudioDestinationNode.h: (WebCore::AudioDestinationNode::resume): (WebCore::AudioDestinationNode::suspend): (WebCore::AudioDestinationNode::close): * Modules/webaudio/DefaultAudioDestinationNode.cpp: (WebCore::DefaultAudioDestinationNode::startRendering): (WebCore::DefaultAudioDestinationNode::resume): (WebCore::DefaultAudioDestinationNode::suspend): (WebCore::DefaultAudioDestinationNode::close): * Modules/webaudio/DefaultAudioDestinationNode.h: * Modules/webaudio/OfflineAudioContext.cpp: (WebCore::OfflineAudioContext::startOfflineRendering): (WebCore::OfflineAudioContext::resumeOfflineRendering): * Modules/webaudio/OfflineAudioDestinationNode.cpp: (WebCore::OfflineAudioDestinationNode::startRendering): * Modules/webaudio/OfflineAudioDestinationNode.h: * platform/audio/AudioDestination.h: (WebCore::AudioDestination::start): (WebCore::AudioDestination::stop): * platform/audio/cocoa/AudioDestinationCocoa.cpp: (WebCore::AudioDestinationCocoa::start): (WebCore::AudioDestinationCocoa::stop): * platform/audio/cocoa/AudioDestinationCocoa.h: * platform/audio/gstreamer/AudioDestinationGStreamer.cpp: (WebCore::AudioDestinationGStreamer::start): (WebCore::AudioDestinationGStreamer::stop): * platform/audio/gstreamer/AudioDestinationGStreamer.h: * platform/mock/MockAudioDestinationCocoa.cpp: (WebCore::MockAudioDestinationCocoa::start): (WebCore::MockAudioDestinationCocoa::stop): * platform/mock/MockAudioDestinationCocoa.h: Source/WebKit: Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination. * GPUProcess/media/RemoteAudioDestinationManager.messages.in: * WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp: (WebKit::RemoteAudioDestinationProxy::start): (WebKit::RemoteAudioDestinationProxy::stop): * WebProcess/GPU/media/RemoteAudioDestinationProxy.h: LayoutTests: * webaudio/audiocontext-state.html: Update existing test which incorrectly expected the AudioContext's state to become "running" synchronously after connecting a source node. The state switches to "running" asynchronously now. It is up to the user agent if and when the audio context starts autoplaying so this should be an acceptable behavior change. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@269073 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent d1614b3 commit 1e5f259

21 files changed

+261
-121
lines changed

‎LayoutTests/ChangeLog

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2020-10-27 Chris Dumez <[email protected]>
2+
3+
[GPUProcess] Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination
4+
https://bugs.webkit.org/show_bug.cgi?id=218251
5+
6+
Reviewed by Geoffrey Garen.
7+
8+
* webaudio/audiocontext-state.html:
9+
Update existing test which incorrectly expected the AudioContext's state to become "running" synchronously
10+
after connecting a source node. The state switches to "running" asynchronously now. It is up to the user
11+
agent if and when the audio context starts autoplaying so this should be an acceptable behavior change.
12+
113
2020-10-27 Fujii Hironori <[email protected]>
214

315
[TextureMapper][GTK] Test compositing/clipping/border-radius-stacking-context-clip.html is failing

‎LayoutTests/webaudio/audiocontext-state.html

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,25 @@
1616
var context = null;
1717
var node = null;
1818

19+
function onAudioContextAutoStart()
20+
{
21+
context.onstatechange = null;
22+
shouldBeEqualToString('context.state', 'running');
23+
24+
debug('Calling context.suspend()');
25+
context.suspend().then(suspendSucceeded, suspendFailed);
26+
}
27+
1928
function runTest() {
2029
window.jsTestIsAsync = true;
2130

2231
context = new AudioContext();
32+
context.onstatechange = onAudioContextAutoStart;
2333

24-
shouldBe('context.state', '"suspended"');
34+
shouldBeEqualToString('context.state', 'suspended');
2535

2636
node = context.createBufferSource();
2737
evalAndLog('node.connect(context.destination)');
28-
29-
shouldBe('context.state', '"running"');
30-
31-
debug('Calling context.suspend()');
32-
context.suspend().then(suspendSucceeded, suspendFailed);
3338
}
3439

3540
function suspendFailed() {

‎Source/WebCore/ChangeLog

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,62 @@
1+
2020-10-27 Chris Dumez <[email protected]>
2+
3+
[GPUProcess] Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination
4+
https://bugs.webkit.org/show_bug.cgi?id=218251
5+
6+
Reviewed by Geoffrey Garen.
7+
8+
DefaultAudioDestinationNode::resume() / suspend() were already asynchronous operations. However, they expected
9+
AudioDestination::start() / stop() to finish synchronously and would simply call their completion handler
10+
asynchronously. Instead, we now make AudioDestination::start() / stop() asynchronous as well. This allows us
11+
to use asynchronous IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination.
12+
13+
As a result of this change, I had to make AudioDestinationNode::startRendering() asynchronous as well since
14+
it uses AudioDestination::start() internally.
15+
16+
As an improvement, the completion handler to AudioDestinationNode's startRendering() / resume() / suspend()
17+
is now provided with an exception in cases where they fail. This allows the call sites to properly deal
18+
with such errors instead of assuming things were successsful.
19+
20+
No new tests, no Web-facing beahvior change.
21+
22+
* Modules/webaudio/AudioContext.cpp:
23+
(WebCore::AudioContext::suspendRendering):
24+
(WebCore::AudioContext::resumeRendering):
25+
(WebCore::AudioContext::startRendering):
26+
(WebCore::AudioContext::mayResumePlayback):
27+
(WebCore::AudioContext::suspendPlayback):
28+
* Modules/webaudio/AudioDestinationNode.h:
29+
(WebCore::AudioDestinationNode::resume):
30+
(WebCore::AudioDestinationNode::suspend):
31+
(WebCore::AudioDestinationNode::close):
32+
* Modules/webaudio/DefaultAudioDestinationNode.cpp:
33+
(WebCore::DefaultAudioDestinationNode::startRendering):
34+
(WebCore::DefaultAudioDestinationNode::resume):
35+
(WebCore::DefaultAudioDestinationNode::suspend):
36+
(WebCore::DefaultAudioDestinationNode::close):
37+
* Modules/webaudio/DefaultAudioDestinationNode.h:
38+
* Modules/webaudio/OfflineAudioContext.cpp:
39+
(WebCore::OfflineAudioContext::startOfflineRendering):
40+
(WebCore::OfflineAudioContext::resumeOfflineRendering):
41+
* Modules/webaudio/OfflineAudioDestinationNode.cpp:
42+
(WebCore::OfflineAudioDestinationNode::startRendering):
43+
* Modules/webaudio/OfflineAudioDestinationNode.h:
44+
* platform/audio/AudioDestination.h:
45+
(WebCore::AudioDestination::start):
46+
(WebCore::AudioDestination::stop):
47+
* platform/audio/cocoa/AudioDestinationCocoa.cpp:
48+
(WebCore::AudioDestinationCocoa::start):
49+
(WebCore::AudioDestinationCocoa::stop):
50+
* platform/audio/cocoa/AudioDestinationCocoa.h:
51+
* platform/audio/gstreamer/AudioDestinationGStreamer.cpp:
52+
(WebCore::AudioDestinationGStreamer::start):
53+
(WebCore::AudioDestinationGStreamer::stop):
54+
* platform/audio/gstreamer/AudioDestinationGStreamer.h:
55+
* platform/mock/MockAudioDestinationCocoa.cpp:
56+
(WebCore::MockAudioDestinationCocoa::start):
57+
(WebCore::MockAudioDestinationCocoa::stop):
58+
* platform/mock/MockAudioDestinationCocoa.h:
59+
160
2020-10-27 Fujii Hironori <[email protected]>
261

362
[TextureMapper][GTK] Test compositing/clipping/border-radius-stacking-context-clip.html is failing

‎Source/WebCore/Modules/webaudio/AudioContext.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,11 @@ void AudioContext::suspendRendering(DOMPromiseDeferred<void>&& promise)
223223

224224
lazyInitialize();
225225

226-
destinationNode()->suspend([this, protectedThis = makeRef(*this), promise = WTFMove(promise)]() mutable {
226+
destinationNode()->suspend([this, protectedThis = makeRef(*this), promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
227+
if (exception) {
228+
promise.reject(WTFMove(*exception));
229+
return;
230+
}
227231
setState(State::Suspended);
228232
promise.resolve();
229233
});
@@ -250,7 +254,12 @@ void AudioContext::resumeRendering(DOMPromiseDeferred<void>&& promise)
250254

251255
lazyInitialize();
252256

253-
destinationNode()->resume([this, protectedThis = makeRef(*this), promise = WTFMove(promise)]() mutable {
257+
destinationNode()->resume([this, protectedThis = makeRef(*this), promise = WTFMove(promise)](Optional<Exception>&& exception) mutable {
258+
if (exception) {
259+
promise.reject(WTFMove(*exception));
260+
return;
261+
}
262+
254263
// Since we update the state asynchronously, we may have been interrupted after the
255264
// call to resume() and before this lambda runs. In this case, we don't want to
256265
// reset the state to running.
@@ -286,10 +295,11 @@ void AudioContext::startRendering()
286295

287296
makePendingActivity();
288297

289-
setState(State::Running);
290-
291298
lazyInitialize();
292-
destination()->startRendering();
299+
destination()->startRendering([this, protectedThis = makeRef(*this)](Optional<Exception>&& exception) {
300+
if (!exception)
301+
setState(State::Running);
302+
});
293303
}
294304

295305
void AudioContext::lazyInitialize()
@@ -357,8 +367,8 @@ void AudioContext::mayResumePlayback(bool shouldResume)
357367

358368
lazyInitialize();
359369

360-
destinationNode()->resume([this, protectedThis = makeRef(*this)] {
361-
setState(State::Running);
370+
destinationNode()->resume([this, protectedThis = makeRef(*this)](Optional<Exception>&& exception) {
371+
setState(exception ? State::Suspended : State::Running);
362372
});
363373
}
364374

@@ -436,7 +446,10 @@ void AudioContext::suspendPlayback()
436446

437447
lazyInitialize();
438448

439-
destinationNode()->suspend([this, protectedThis = makeRef(*this)] {
449+
destinationNode()->suspend([this, protectedThis = makeRef(*this)](Optional<Exception>&& exception) {
450+
if (exception)
451+
return;
452+
440453
bool interrupted = m_mediaSession->state() == PlatformMediaSession::Interrupted;
441454
setState(interrupted ? State::Interrupted : State::Suspended);
442455
});

‎Source/WebCore/Modules/webaudio/AudioDestinationNode.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include "AudioIOCallback.h"
2929
#include "AudioNode.h"
3030
#include "ExceptionOr.h"
31-
#include <wtf/Function.h>
31+
#include <wtf/CompletionHandler.h>
3232

3333
namespace WebCore {
3434

@@ -53,10 +53,10 @@ class AudioDestinationNode : public AudioNode, public AudioIOCallback {
5353
// Enable local/live input for the specified device.
5454
virtual void enableInput(const String& inputDeviceId) = 0;
5555

56-
virtual ExceptionOr<void> startRendering() = 0;
57-
virtual void resume(WTF::Function<void ()>&&) { }
58-
virtual void suspend(WTF::Function<void ()>&&) { }
59-
virtual void close(WTF::Function<void ()>&&) { }
56+
virtual void startRendering(CompletionHandler<void(Optional<Exception>&&)>&&) = 0;
57+
virtual void resume(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler) { completionHandler(WTF::nullopt); }
58+
virtual void suspend(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler) { completionHandler(WTF::nullopt); }
59+
virtual void close(CompletionHandler<void()>&& completionHandler) { completionHandler(); }
6060

6161
virtual bool isPlaying() { return false; }
6262
void isPlayingDidChange() override;

‎Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -119,37 +119,53 @@ Function<void(Function<void()>&&)> DefaultAudioDestinationNode::dispatchToRender
119119
return { };
120120
}
121121

122-
ExceptionOr<void> DefaultAudioDestinationNode::startRendering()
122+
void DefaultAudioDestinationNode::startRendering(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler)
123123
{
124124
ASSERT(isInitialized());
125125
if (!isInitialized())
126-
return Exception { InvalidStateError, "AudioDestinationNode is not initialized"_s };
126+
return completionHandler(Exception { InvalidStateError, "AudioDestinationNode is not initialized"_s });
127127

128-
m_destination->start(dispatchToRenderThreadFunction());
129-
return { };
128+
auto innerCompletionHandler = [completionHandler = WTFMove(completionHandler)](bool success) mutable {
129+
completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start the audio device"_s }));
130+
};
131+
132+
m_destination->start(dispatchToRenderThreadFunction(), WTFMove(innerCompletionHandler));
130133
}
131134

132-
void DefaultAudioDestinationNode::resume(Function<void ()>&& function)
135+
void DefaultAudioDestinationNode::resume(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler)
133136
{
134137
ASSERT(isInitialized());
135-
if (isInitialized())
136-
m_destination->start(dispatchToRenderThreadFunction());
137-
context().postTask(WTFMove(function));
138+
if (!isInitialized()) {
139+
context().postTask([completionHandler = WTFMove(completionHandler)]() mutable {
140+
completionHandler(Exception { InvalidStateError, "AudioDestinationNode is not initialized"_s });
141+
});
142+
return;
143+
}
144+
m_destination->start(dispatchToRenderThreadFunction(), [completionHandler = WTFMove(completionHandler)](bool success) mutable {
145+
completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start the audio device"_s }));
146+
});
138147
}
139148

140-
void DefaultAudioDestinationNode::suspend(Function<void ()>&& function)
149+
void DefaultAudioDestinationNode::suspend(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler)
141150
{
142151
ASSERT(isInitialized());
143-
if (isInitialized())
144-
m_destination->stop();
145-
context().postTask(WTFMove(function));
152+
if (!isInitialized()) {
153+
context().postTask([completionHandler = WTFMove(completionHandler)]() mutable {
154+
completionHandler(Exception { InvalidStateError, "AudioDestinationNode is not initialized"_s });
155+
});
156+
return;
157+
}
158+
159+
m_destination->stop([completionHandler = WTFMove(completionHandler)](bool success) mutable {
160+
completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop the audio device"_s }));
161+
});
146162
}
147163

148-
void DefaultAudioDestinationNode::close(Function<void()>&& function)
164+
void DefaultAudioDestinationNode::close(CompletionHandler<void()>&& completionHandler)
149165
{
150166
ASSERT(isInitialized());
151167
uninitialize();
152-
context().postTask(WTFMove(function));
168+
context().postTask(WTFMove(completionHandler));
153169
}
154170

155171
unsigned DefaultAudioDestinationNode::maxChannelCount() const

‎Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class DefaultAudioDestinationNode final : public AudioDestinationNode {
4343
unsigned framesPerBuffer() const;
4444
float sampleRate() const final { return m_sampleRate; }
4545

46-
ExceptionOr<void> startRendering() final;
46+
void startRendering(CompletionHandler<void(Optional<Exception>&&)>&&) final;
4747

4848
private:
4949
explicit DefaultAudioDestinationNode(BaseAudioContext&, Optional<float>);
@@ -58,9 +58,9 @@ class DefaultAudioDestinationNode final : public AudioDestinationNode {
5858
bool requiresTailProcessing() const final { return false; }
5959

6060
void enableInput(const String& inputDeviceId) final;
61-
void resume(Function<void ()>&&) final;
62-
void suspend(Function<void ()>&&) final;
63-
void close(Function<void ()>&&) final;
61+
void resume(CompletionHandler<void(Optional<Exception>&&)>&&) final;
62+
void suspend(CompletionHandler<void(Optional<Exception>&&)>&&) final;
63+
void close(CompletionHandler<void()>&&) final;
6464
unsigned maxChannelCount() const final;
6565
bool isPlaying() final;
6666

‎Source/WebCore/Modules/webaudio/OfflineAudioContext.cpp

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,17 @@ void OfflineAudioContext::startOfflineRendering(Ref<DeferredPromise>&& promise)
105105

106106
lazyInitialize();
107107

108-
auto result = destination()->startRendering();
109-
if (result.hasException()) {
110-
promise->reject(result.releaseException());
111-
return;
112-
}
113-
114-
makePendingActivity();
115-
m_pendingOfflineRenderingPromise = WTFMove(promise);
116-
m_didStartOfflineRendering = true;
117-
setState(State::Running);
108+
destination()->startRendering([this, promise = WTFMove(promise), pendingActivity = ActiveDOMObject::makePendingActivity(*this)](Optional<Exception>&& exception) mutable {
109+
if (exception) {
110+
promise->reject(WTFMove(*exception));
111+
return;
112+
}
113+
114+
makePendingActivity();
115+
m_pendingOfflineRenderingPromise = WTFMove(promise);
116+
m_didStartOfflineRendering = true;
117+
setState(State::Running);
118+
});
118119
}
119120

120121
void OfflineAudioContext::suspendOfflineRendering(double suspendTime, Ref<DeferredPromise>&& promise)
@@ -166,15 +167,16 @@ void OfflineAudioContext::resumeOfflineRendering(Ref<DeferredPromise>&& promise)
166167
}
167168
ASSERT(state() == AudioContextState::Suspended);
168169

169-
auto result = destination()->startRendering();
170-
if (result.hasException()) {
171-
promise->reject(result.releaseException());
172-
return;
173-
}
170+
destination()->startRendering([this, promise = WTFMove(promise), pendingActivity = ActiveDOMObject::makePendingActivity(*this)](Optional<Exception>&& exception) mutable {
171+
if (exception) {
172+
promise->reject(WTFMove(*exception));
173+
return;
174+
}
174175

175-
makePendingActivity();
176-
setState(State::Running);
177-
promise->resolve();
176+
makePendingActivity();
177+
setState(State::Running);
178+
promise->resolve();
179+
});
178180
}
179181

180182
bool OfflineAudioContext::shouldSuspend()

‎Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,17 @@ void OfflineAudioDestinationNode::uninitialize()
8484
AudioNode::uninitialize();
8585
}
8686

87-
ExceptionOr<void> OfflineAudioDestinationNode::startRendering()
87+
void OfflineAudioDestinationNode::startRendering(CompletionHandler<void(Optional<Exception>&&)>&& completionHandler)
8888
{
8989
ALWAYS_LOG(LOGIDENTIFIER);
9090

9191
ASSERT(isMainThread());
9292
ASSERT(m_renderTarget.get());
9393
if (!m_renderTarget.get())
94-
return Exception { InvalidStateError, "OfflineAudioContextNode has no rendering buffer"_s };
94+
return completionHandler(Exception { InvalidStateError, "OfflineAudioContextNode has no rendering buffer"_s });
9595

9696
if (m_startedRendering)
97-
return Exception { InvalidStateError, "Already started rendering"_s };
97+
return completionHandler(Exception { InvalidStateError, "Already started rendering"_s });
9898

9999
m_startedRendering = true;
100100
auto protectedThis = makeRef(*this);
@@ -121,12 +121,12 @@ ExceptionOr<void> OfflineAudioDestinationNode::startRendering()
121121
workletProxy->postTaskForModeToWorkletGlobalScope([offThreadRendering = WTFMove(offThreadRendering)](ScriptExecutionContext&) mutable {
122122
offThreadRendering();
123123
}, WorkerRunLoop::defaultMode());
124-
return { };
124+
return completionHandler(WTF::nullopt);
125125
}
126126

127127
// FIXME: We should probably limit the number of threads we create for offline audio.
128128
m_renderThread = Thread::create("offline renderer", WTFMove(offThreadRendering), ThreadType::Audio);
129-
return { };
129+
completionHandler(WTF::nullopt);
130130
}
131131

132132
auto OfflineAudioDestinationNode::offlineRender() -> OfflineRenderResult

‎Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class OfflineAudioDestinationNode final : public AudioDestinationNode {
5050

5151
// AudioDestinationNode
5252
void enableInput(const String&) override { }
53-
ExceptionOr<void> startRendering() final;
53+
void startRendering(CompletionHandler<void(Optional<Exception>&&)>&&) final;
5454

5555
float sampleRate() const final { return m_renderTarget->sampleRate(); }
5656

0 commit comments

Comments
 (0)