Skip to content

Commit 9c7c6b9

Browse files
youennfyouennf
committed
Calling getUserMedia with echo cancellation disabled causes side effects for existing audio tracks acquired via getUserMedia
rdar://151143554 https://bugs.webkit.org/show_bug.cgi?id=292861 Reviewed by Jean-Yves Avenard. On macOS, we can run one VPIO unit and several HAL units in parallel. This allows to use at the same time a microphone without echo cancellation (using HAL units) and with echo cancellation (using VPIO). To do so, CoreAudioCaptureSource has its own CoreAudioCaptureUnit whenever echo cancellation is off. CoreAudioCaptureSource uses CoreAudioCaptureUnit::defaultSingleton whenever echo cancellation is on. We update BaseAudioCaptureUnit to take a boolean telling whether it supports echo cancellation. This is used in BaseAudioCaptureUnit::setEnableEchoCancellation to validate we are not creating more than one VPIO unit. CoreAudioCaptureSource, just before initializing its audio unit, will select its CoreAudioCaptureUnit based on echo cancellation. The same principle is also done in CoreAudioCaptureSource::settingsDidChange, which happens when the web page calls MediaStreamTrack.applyConstraints. The code path used for applyConstraints reuse the initialization code path, which requires to reset m_shouldInitializeAudioUnit to true. It is possible to use applyConstraints to switch between HAL and VPIO units but all track clones are sharing the same CoreAudioCaptureUnit. To get independent tracks, the web page will have to call getUserMedia twice. A future patch should fix this limitation. This is specific to macOS, no change to iOS behavior. Now that we are creating potentially more than one CoreAudioCaptureUnit, we need to mock all of them. We update our mock code so that we can iterate on existing CoreAudioCaptureUnit and update newly created. Covered by added test. Canonical link: https://commits.webkit.org/302582@main
1 parent 8bc3a0e commit 9c7c6b9

13 files changed

+193
-24
lines changed

‎LayoutTests/fast/mediastream/mediastreamtrack-configurationchange-2.html‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414

1515
assert_true(track.getSettings().echoCancellation);
1616

17+
// Currently, iOS platforms do not support capturing audio with and without echo cancellation.
18+
if (!window.internals || internals.supportsMultiMicrophoneCaptureWithoutEchoCancellation())
19+
return;
20+
1721
let promise = new Promise((resolve, reject) => {
1822
track.onconfigurationchange = resolve;
1923
setTimeout(()=> reject("waited too long for configurationchange"), 5000);
@@ -28,12 +32,11 @@
2832
await promise;
2933
assert_false(track.getSettings().echoCancellation, "track");
3034

31-
promise = new Promise((resolve, reject) => {
35+
promise = new Promise((resolve, reject) => {
3236
track2.onconfigurationchange = resolve;
3337
setTimeout(()=> reject("waited too long for configurationchange2"), 5000);
3438
});
3539

36-
// Currently, cocoa platforms do not support capturing audio with and without echo cancellation.
3740
track.applyConstraints({echoCancellation: true});
3841

3942
await promise;

‎LayoutTests/fast/mediastream/microphone-with-and-without-echocancellation-expected.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
PASS Capturing without echo cancellation shouldn't affect echo cancelled tracks of the same device
3+
PASS Capturing with echo cancellation shouldn't affect tracks without echo cancellation of the same device
4+
PASS Using applyConstraints for enabling/disabling echo cancellation should not interfere with another microphone track
5+
FAIL Using applyConstraints for enabling/disabling echo cancellation should not interfere with another cloned track assert_true: FIXME: support changing settings of cloned audio tracks - expected true got false
6+

‎LayoutTests/fast/mediastream/microphone-with-and-without-echocancellation.html‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<script src="../../resources/testharness.js"></script>
2+
<script src="../../resources/testharnessreport.js"></script>
3+
<script>
4+
promise_test(async (t) => {
5+
const stream1 = await navigator.mediaDevices.getUserMedia({ audio: true });
6+
const track1 = stream1.getAudioTracks()[0];
7+
t.add_cleanup(() => track1.stop());
8+
assert_true(track1.getSettings().echoCancellation, "test1");
9+
10+
const stream2 = await navigator.mediaDevices.getUserMedia({ audio: { echoCancellation:false } });
11+
const track2 = stream2.getAudioTracks()[0];
12+
t.add_cleanup(() => track2.stop());
13+
assert_false(track2.getSettings().echoCancellation, "test2");
14+
15+
if (window.internals && !internals.supportsMultiMicrophoneCaptureWithoutEchoCancellation())
16+
return;
17+
18+
await new Promise(resolve => t.step_timeout(resolve, 50));
19+
assert_true(track1.getSettings().echoCancellation, "test3");
20+
}, "Capturing without echo cancellation shouldn't affect echo cancelled tracks of the same device");
21+
22+
promise_test(async (t) => {
23+
const stream1 = await navigator.mediaDevices.getUserMedia({ audio: { echoCancellation:false } });
24+
const track1 = stream1.getAudioTracks()[0];
25+
t.add_cleanup(() => track1.stop());
26+
assert_false(track1.getSettings().echoCancellation, "test1");
27+
28+
const stream2 = await navigator.mediaDevices.getUserMedia({ audio: true });
29+
const track2 = stream2.getAudioTracks()[0];
30+
t.add_cleanup(() => track2.stop());
31+
assert_true(track2.getSettings().echoCancellation, "test2");
32+
33+
if (window.internals && !internals.supportsMultiMicrophoneCaptureWithoutEchoCancellation())
34+
return;
35+
36+
await new Promise(resolve => t.step_timeout(resolve, 50));
37+
assert_false(track1.getSettings().echoCancellation, "test3");
38+
}, "Capturing with echo cancellation shouldn't affect tracks without echo cancellation of the same device");
39+
40+
promise_test(async (t) => {
41+
const stream1 = await navigator.mediaDevices.getUserMedia({ audio: { echoCancellation:false } });
42+
const track1 = stream1.getAudioTracks()[0];
43+
t.add_cleanup(() => track1.stop());
44+
assert_false(track1.getSettings().echoCancellation, "test1");
45+
46+
const stream2 = await navigator.mediaDevices.getUserMedia({ audio: true });
47+
const track2 = stream2.getAudioTracks()[0];
48+
t.add_cleanup(() => track2.stop());
49+
assert_true(track2.getSettings().echoCancellation, "test2");
50+
51+
if (window.internals && !internals.supportsMultiMicrophoneCaptureWithoutEchoCancellation())
52+
return;
53+
54+
await track1.applyConstraints({ echoCancellation:true });
55+
assert_true(track1.getSettings().echoCancellation, "test3");
56+
assert_true(track2.getSettings().echoCancellation, "test4");
57+
58+
await track1.applyConstraints({ echoCancellation:false });
59+
assert_false(track1.getSettings().echoCancellation, "test5");
60+
assert_true(track2.getSettings().echoCancellation, "test6");
61+
}, "Using applyConstraints for enabling/disabling echo cancellation should not interfere with another microphone track");
62+
63+
promise_test(async (t) => {
64+
const stream1 = await navigator.mediaDevices.getUserMedia({ audio: { echoCancellation:false } });
65+
const track1 = stream1.getAudioTracks()[0];
66+
t.add_cleanup(() => track1.stop());
67+
assert_false(track1.getSettings().echoCancellation, "test1");
68+
69+
const track2 = track1.clone();
70+
t.add_cleanup(() => track2.stop());
71+
assert_false(track2.getSettings().echoCancellation, "test2");
72+
73+
await track1.applyConstraints({ echoCancellation:true });
74+
assert_true(track1.getSettings().echoCancellation, "test3");
75+
assert_true(!track2.getSettings().echoCancellation, "FIXME: support changing settings of cloned audio tracks -");
76+
}, "Using applyConstraints for enabling/disabling echo cancellation should not interfere with another cloned track");
77+
</script>

‎Source/WebCore/platform/mediastream/mac/BaseAudioCaptureUnit.cpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ namespace WebCore {
4040

4141
constexpr Seconds voiceActivityThrottlingDuration = 5_s;
4242

43-
BaseAudioCaptureUnit::BaseAudioCaptureUnit()
44-
: m_sampleRate(AudioSession::singleton().sampleRate())
43+
BaseAudioCaptureUnit::BaseAudioCaptureUnit(CanEnableEchoCancellation canEnableEchoCancellation)
44+
: m_canEnableEchoCancellation(canEnableEchoCancellation == CanEnableEchoCancellation::Yes)
45+
, m_enableEchoCancellation(m_canEnableEchoCancellation)
46+
, m_sampleRate(AudioSession::singleton().sampleRate())
4547
{
4648
RealtimeMediaSourceCenter::singleton().addDevicesChangedObserver(*this);
4749
}
@@ -51,6 +53,12 @@ BaseAudioCaptureUnit::~BaseAudioCaptureUnit()
5153
RealtimeMediaSourceCenter::singleton().removeDevicesChangedObserver(*this);
5254
}
5355

56+
void BaseAudioCaptureUnit::setEnableEchoCancellation(bool enableEchoCancellation)
57+
{
58+
ASSERT(m_canEnableEchoCancellation || (!enableEchoCancellation && !m_enableEchoCancellation));
59+
m_enableEchoCancellation = enableEchoCancellation;
60+
}
61+
5462
void BaseAudioCaptureUnit::addClient(CoreAudioCaptureSource& client)
5563
{
5664
ASSERT(isMainThread());

‎Source/WebCore/platform/mediastream/mac/BaseAudioCaptureUnit.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class BaseAudioCaptureUnit : public RefCounted<BaseAudioCaptureUnit>, public Rea
7777

7878
void setVolume(double volume) { m_volume = volume; }
7979
void setSampleRate(int sampleRate) { m_sampleRate = sampleRate; }
80-
void setEnableEchoCancellation(bool enableEchoCancellation) { m_enableEchoCancellation = enableEchoCancellation; }
80+
void setEnableEchoCancellation(bool);
8181

8282
void addClient(CoreAudioCaptureSource&);
8383
void removeClient(CoreAudioCaptureSource&);
@@ -103,7 +103,8 @@ class BaseAudioCaptureUnit : public RefCounted<BaseAudioCaptureUnit>, public Rea
103103
#endif
104104

105105
protected:
106-
BaseAudioCaptureUnit();
106+
enum class CanEnableEchoCancellation : bool { No, Yes };
107+
explicit BaseAudioCaptureUnit(CanEnableEchoCancellation);
107108

108109
void forEachClient(NOESCAPE const Function<void(CoreAudioCaptureSource&)>&) const;
109110
void captureFailed();
@@ -153,6 +154,7 @@ class BaseAudioCaptureUnit : public RefCounted<BaseAudioCaptureUnit>, public Rea
153154
void continueStartProducingData();
154155
void continueStopProducingData();
155156

157+
const bool m_canEnableEchoCancellation { true };
156158
bool m_enableEchoCancellation { true };
157159
double m_volume { 1 };
158160
int m_sampleRate;

‎Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,16 @@ Ref<const CoreAudioCaptureUnit> CoreAudioCaptureSource::protectedUnit() const
224224

225225
void CoreAudioCaptureSource::initializeToStartProducingData()
226226
{
227-
if (m_isReadyToStart)
227+
if (!m_shouldInitializeAudioUnit)
228228
return;
229229

230230
ALWAYS_LOG_IF(loggerPtr(), LOGIDENTIFIER, "is Default ", captureDevice().isDefault());
231-
m_isReadyToStart = true;
231+
m_shouldInitializeAudioUnit = false;
232+
233+
#if PLATFORM(MAC)
234+
if (echoCancellation() != protectedUnit()->enableEchoCancellation())
235+
m_unit = echoCancellation() ? Ref { CoreAudioCaptureUnit::defaultSingleton() } : CoreAudioCaptureUnit::createNonVPIOUnit();
236+
#endif
232237

233238
Ref unit = m_unit;
234239
unit->setCaptureDevice(String { persistentID() }, m_captureDeviceID, captureDevice().isDefault());
@@ -317,15 +322,28 @@ const RealtimeMediaSourceSettings& CoreAudioCaptureSource::settings()
317322

318323
void CoreAudioCaptureSource::settingsDidChange(OptionSet<RealtimeMediaSourceSettings::Flag> settings)
319324
{
320-
if (!m_isReadyToStart || m_echoCancellationChanging) {
325+
if (m_shouldInitializeAudioUnit || m_echoCancellationChanging) {
321326
m_currentSettings = std::nullopt;
322327
return;
323328
}
324329

325330
bool shouldReconfigure = false;
326331
if (settings.contains(RealtimeMediaSourceSettings::Flag::EchoCancellation)) {
332+
#if PLATFORM(MAC)
333+
Ref unit = m_unit;
334+
unit->removeClient(*this);
335+
if (isProducingData())
336+
unit->stopProducingData();
337+
338+
m_unit = echoCancellation() ? Ref { CoreAudioCaptureUnit::defaultSingleton() } : CoreAudioCaptureUnit::createNonVPIOUnit();
339+
m_shouldInitializeAudioUnit = true;
340+
if (isProducingData())
341+
startProducingData();
342+
return;
343+
#else
327344
protectedUnit()->setEnableEchoCancellation(echoCancellation());
328345
shouldReconfigure = true;
346+
#endif
329347
}
330348
if (settings.contains(RealtimeMediaSourceSettings::Flag::SampleRate)) {
331349
protectedUnit()->setSampleRate(sampleRate());

‎Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class CoreAudioCaptureSource : public RealtimeMediaSource, public ThreadSafeRefC
116116
std::optional<RealtimeMediaSourceSettings> m_currentSettings;
117117

118118
bool m_canResumeAfterInterruption { true };
119-
bool m_isReadyToStart { false };
119+
bool m_shouldInitializeAudioUnit { true };
120120
bool m_echoCancellationChanging { false };
121121

122122
std::optional<bool> m_echoCancellationCapability;

‎Source/WebCore/platform/mediastream/mac/CoreAudioCaptureUnit.cpp‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,19 @@ OSStatus CoreAudioCaptureInternalUnit::defaultOutputDevice(uint32_t* deviceID)
215215

216216
CoreAudioCaptureUnit& CoreAudioCaptureUnit::defaultSingleton()
217217
{
218-
static NeverDestroyed<Ref<CoreAudioCaptureUnit>> singleton(adoptRef(*new CoreAudioCaptureUnit));
218+
static NeverDestroyed<Ref<CoreAudioCaptureUnit>> singleton(adoptRef(*new CoreAudioCaptureUnit(CanEnableEchoCancellation::Yes)));
219219
return singleton.get();
220220
}
221221

222+
Ref<CoreAudioCaptureUnit> CoreAudioCaptureUnit::createNonVPIOUnit()
223+
{
224+
return adoptRef(*new CoreAudioCaptureUnit(CanEnableEchoCancellation::No));
225+
}
226+
222227
static WeakHashSet<CoreAudioCaptureUnit>& allCoreAudioCaptureUnits()
223228
{
229+
ASSERT(isMainThread());
230+
224231
static NeverDestroyed<WeakHashSet<CoreAudioCaptureUnit>> units;
225232
return units;
226233
}
@@ -230,13 +237,29 @@ void CoreAudioCaptureUnit::forEach(NOESCAPE Function<void(CoreAudioCaptureUnit&)
230237
allCoreAudioCaptureUnits().forEach(WTFMove(callback));
231238
}
232239

233-
CoreAudioCaptureUnit::CoreAudioCaptureUnit()
234-
: m_sampleRateCapabilities(8000, 96000)
240+
static Function<void(CoreAudioCaptureUnit&)>& coreAudioCaptureNewUnitCallback()
241+
{
242+
ASSERT(isMainThread());
243+
244+
static NeverDestroyed<Function<void(CoreAudioCaptureUnit&)>> callback;
245+
return callback.get();
246+
}
247+
248+
void CoreAudioCaptureUnit::forNewUnit(Function<void(CoreAudioCaptureUnit&)>&& callback)
249+
{
250+
coreAudioCaptureNewUnitCallback() = WTFMove(callback);
251+
}
252+
253+
CoreAudioCaptureUnit::CoreAudioCaptureUnit(CanEnableEchoCancellation canEnableEchoCancellation)
254+
: BaseAudioCaptureUnit(canEnableEchoCancellation)
255+
, m_sampleRateCapabilities(8000, 96000)
235256
#if PLATFORM(MAC)
236257
, m_storedVPIOUnitDeallocationTimer(*this, &CoreAudioCaptureUnit::deallocateStoredVPIOUnit)
237258
#endif
238259
{
239260
allCoreAudioCaptureUnits().add(*this);
261+
if (coreAudioCaptureNewUnitCallback())
262+
coreAudioCaptureNewUnitCallback()(*this);
240263
}
241264

242265
CoreAudioCaptureUnit::~CoreAudioCaptureUnit()

‎Source/WebCore/platform/mediastream/mac/CoreAudioCaptureUnit.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,12 @@ class CoreAudioCaptureUnit final : public BaseAudioCaptureUnit {
8383

8484
// The default unit - the only one that may render audio when capturing
8585
WEBCORE_EXPORT static CoreAudioCaptureUnit& defaultSingleton();
86-
WEBCORE_EXPORT static void forEach(NOESCAPE Function<void(CoreAudioCaptureUnit&)>&&);
86+
static Ref<CoreAudioCaptureUnit> createNonVPIOUnit();
8787
~CoreAudioCaptureUnit();
8888

89+
WEBCORE_EXPORT static void forEach(NOESCAPE Function<void(CoreAudioCaptureUnit&)>&&);
90+
static void forNewUnit(Function<void(CoreAudioCaptureUnit&)>&&);
91+
8992
using CreationCallback = Function<Expected<UniqueRef<InternalUnit>, OSStatus>(bool enableEchoCancellation)>;
9093
void setInternalUnitCreationCallback(CreationCallback&& callback) { m_creationCallback = WTFMove(callback); }
9194
using GetSampleRateCallback = Function<int()>;
@@ -132,7 +135,7 @@ class CoreAudioCaptureUnit final : public BaseAudioCaptureUnit {
132135
void delaySamples(Seconds) final;
133136

134137
private:
135-
CoreAudioCaptureUnit();
138+
explicit CoreAudioCaptureUnit(CanEnableEchoCancellation);
136139

137140
friend class NeverDestroyed<CoreAudioCaptureUnit>;
138141
friend class MockAudioCaptureInternalUnit;

‎Source/WebCore/platform/mediastream/mac/MockAudioCaptureUnit.mm‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,27 +164,45 @@ static void addHum(float amplitude, float frequency, float sampleRate, uint64_t
164164
AURenderCallbackStruct m_speakerCallback;
165165
};
166166

167+
static void mockAudioUnit(CoreAudioCaptureUnit& unit)
168+
{
169+
unit.setSampleRateRange({ 44100, 96000 });
170+
unit.setInternalUnitCreationCallback([](bool enableEchoCancellation) {
171+
UniqueRef<CoreAudioCaptureUnit::InternalUnit> result = makeUniqueRef<MockAudioCaptureInternalUnit>(enableEchoCancellation);
172+
return result;
173+
});
174+
unit.setInternalUnitGetSampleRateCallback([] { return 44100; });
175+
}
176+
177+
static void unmockAudioUnit(CoreAudioCaptureUnit& unit)
178+
{
179+
unit.setSampleRateRange({ 8000, 96000 });
180+
unit.setInternalUnitCreationCallback({ });
181+
unit.setInternalUnitGetSampleRateCallback({ });
182+
unit.setSampleRateRange({ 44100, 96000 });
183+
}
184+
167185
static bool s_shouldIncreaseBufferSize;
168186
void MockAudioCaptureUnit::enable()
169187
{
170188
s_shouldIncreaseBufferSize = false;
171-
CoreAudioCaptureUnit::defaultSingleton().setSampleRateRange({ 44100, 96000 });
172-
CoreAudioCaptureUnit::defaultSingleton().setInternalUnitCreationCallback([](bool enableEchoCancellation) {
173-
UniqueRef<CoreAudioCaptureUnit::InternalUnit> result = makeUniqueRef<MockAudioCaptureInternalUnit>(enableEchoCancellation);
174-
return result;
189+
CoreAudioCaptureUnit::forEach([](auto& unit) {
190+
mockAudioUnit(unit);
175191
});
176-
CoreAudioCaptureUnit::defaultSingleton().setInternalUnitGetSampleRateCallback([] {
177-
return 44100;
192+
CoreAudioCaptureUnit::forNewUnit([](auto& unit) {
193+
mockAudioUnit(unit);
178194
});
179195
}
180196

181197
void MockAudioCaptureUnit::disable()
182198
{
183-
CoreAudioCaptureUnit::defaultSingleton().setSampleRateRange({ 8000, 96000 });
184-
CoreAudioCaptureUnit::defaultSingleton().setInternalUnitCreationCallback({ });
185-
CoreAudioCaptureUnit::defaultSingleton().setInternalUnitGetSampleRateCallback({ });
199+
CoreAudioCaptureUnit::forEach([](auto& unit) {
200+
unmockAudioUnit(unit);
201+
});
202+
CoreAudioCaptureUnit::forNewUnit({ });
186203
}
187204

205+
188206
void MockAudioCaptureUnit::increaseBufferSize()
189207
{
190208
s_shouldIncreaseBufferSize = true;

0 commit comments

Comments
 (0)