Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set echoCancellation to true if not explicitly set within a getUserMedia call #14490

Merged
merged 1 commit into from
May 31, 2023
Merged

Set echoCancellation to true if not explicitly set within a getUserMedia call #14490

merged 1 commit into from
May 31, 2023

Conversation

youennf
Copy link
Contributor

@youennf youennf commented May 30, 2023 •
edited by webkit-early-warning-system

3fb1003

Set echoCancellation to true if not explicitly set within a getUserMedia call
https://bugs.webkit.org/show_bug.cgi?id=257495
rdar://110010294

Reviewed by Eric Carlson.

Before the patch, if the web page does not provide a echoCancellation setting, we would use whatever setting was used by the previous web application using the microphone.
This might break some applications that want echoCancellation but do not set it explicitly.
This change aligns with Chrome's behavior.

* Source/WebCore/Modules/mediastream/MediaDevices.cpp:
(WebCore::MediaDevices::getUserMedia):
* Source/WebCore/platform/mediastream/MediaConstraints.cpp:
(WebCore::MediaConstraints::setDefaultAudioConstraints):
* Source/WebCore/platform/mediastream/MediaConstraints.h:
* LayoutTests/fast/mediastream/getUserMedia-echoCancellation-expected.txt: Added.
* LayoutTests/fast/mediastream/getUserMedia-echoCancellation.html: Added.

Canonical link: https://commits.webkit.org/264721@main

5bdab04

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv   🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@youennf youennf self-assigned this May 30, 2023
@youennf youennf added the WebRTC For bugs in WebRTC label May 30, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash c4b32d3)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
❌ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@youennf youennf requested a review from eric-carlson May 30, 2023 16:59
@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label May 31, 2023
@webkit-commit-queue
Copy link
Collaborator

Commit message contains (OOPS!), blocking PR #14490

@webkit-commit-queue webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels May 31, 2023
@youennf youennf removed the merging-blocked Applied to prevent a change from being merged label May 31, 2023
@youennf youennf force-pushed the eng/Set-echoCancellation-to-true-if-not-explicitly-set-within-a-getUserMedia-call branch from c4b32d3 to 5bdab04 Compare May 31, 2023 07:17
@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label May 31, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 5bdab04)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv   🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@youennf
Set echoCancellation to true if not explicitly set within a getUserMe…
3fb1003 
…dia call

https://bugs.webkit.org/show_bug.cgi?id=257495
rdar://110010294

Reviewed by Eric Carlson.

Before the patch, if the web page does not provide a echoCancellation setting, we would use whatever setting was used by the previous web application using the microphone.
This might break some applications that want echoCancellation but do not set it explicitly.
This change aligns with Chrome's behavior.

* Source/WebCore/Modules/mediastream/MediaDevices.cpp:
(WebCore::MediaDevices::getUserMedia):
* Source/WebCore/platform/mediastream/MediaConstraints.cpp:
(WebCore::MediaConstraints::setDefaultAudioConstraints):
* Source/WebCore/platform/mediastream/MediaConstraints.h:
* LayoutTests/fast/mediastream/getUserMedia-echoCancellation-expected.txt: Added.
* LayoutTests/fast/mediastream/getUserMedia-echoCancellation.html: Added.

Canonical link: https://commits.webkit.org/264721@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Set-echoCancellation-to-true-if-not-explicitly-set-within-a-getUserMedia-call branch from 5bdab04 to 3fb1003 Compare May 31, 2023 08:31
@webkit-commit-queue
Copy link
Collaborator

Committed 264721@main (3fb1003): https://commits.webkit.org/264721@main

Reviewed commits have been landed. Closing PR #14490 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 3fb1003 into WebKit:main May 31, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-carlson eric-carlson eric-carlson approved these changes

Assignees

@youennf youennf

Labels
WebRTC For bugs in WebRTC
Projects
None yet
Milestone
No milestone
4 participants
@youennf @webkit-early-warning-system @webkit-commit-queue @eric-carlson