Skip to content

MediaElementAudioSourceNode should account for preservesPitch when resampling for playback rate#61548

Closed
Ahmad-S792 wants to merge 1 commit into
WebKit:mainfrom
Ahmad-S792:eng/MediaElementAudioSourceNode-should-account-for-preservesPitch-when-resampling-for-playback-rate
Closed

MediaElementAudioSourceNode should account for preservesPitch when resampling for playback rate#61548
Ahmad-S792 wants to merge 1 commit into
WebKit:mainfrom
Ahmad-S792:eng/MediaElementAudioSourceNode-should-account-for-preservesPitch-when-resampling-for-playback-rate

Conversation

@Ahmad-S792
Copy link
Copy Markdown
Contributor

@Ahmad-S792 Ahmad-S792 commented Mar 28, 2026

46cbc7d

MediaElementAudioSourceNode should account for preservesPitch when resampling for playback rate
https://bugs.webkit.org/show_bug.cgi?id=310951
rdar://173564158

Reviewed by NOBODY (OOPS!).

The commit 310098@main added playback rate resampling in
MediaElementAudioSourceNode so that pitch shifts correctly when
preservesPitch=false and the element is connected to an AudioContext
via createMediaElementSource(). However, the resampling was applied
unconditionally, which also shifted pitch when preservesPitch=true.

The MTAudioProcessingTap captures decoded audio before AVFoundation's
time-pitch algorithm runs, so the AudioContext always receives the
original pitch. The resampler must therefore only apply the playback
rate factor when preservesPitch is false; when true, the user expects
the original pitch regardless of playback rate.

Add setPreservesPitch() to MediaElementAudioSourceNode and have
HTMLMediaElement notify it when the flag changes. In
updateResamplerIfNeeded(), use the playback rate as a resampling
factor only when preservesPitch is false.

With 310098@main, we progressed two subtests in `preserves-pitch.html`
WPT but regressed two as well (preservesPitch=true) cases. Now, this
patch fixes them and unskip whole test altogether.

* LayoutTests/TestExpectations: Unskip (if unskipped, it would have caught the issue before)
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/preserves-pitch-expected.txt:
* Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:
(WebCore::MediaElementAudioSourceNode::setPreservesPitch):
(WebCore::MediaElementAudioSourceNode::updateResamplerIfNeeded):
* Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.h:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setPreservesPitch):

46cbc7d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ⏳ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests loading 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ❌ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

…sampling for playback rate

https://bugs.webkit.org/show_bug.cgi?id=310951
rdar://173564158

Reviewed by NOBODY (OOPS!).

The commit 310098@main added playback rate resampling in
MediaElementAudioSourceNode so that pitch shifts correctly when
preservesPitch=false and the element is connected to an AudioContext
via createMediaElementSource(). However, the resampling was applied
unconditionally, which also shifted pitch when preservesPitch=true.

The MTAudioProcessingTap captures decoded audio before AVFoundation's
time-pitch algorithm runs, so the AudioContext always receives the
original pitch. The resampler must therefore only apply the playback
rate factor when preservesPitch is false; when true, the user expects
the original pitch regardless of playback rate.

Add setPreservesPitch() to MediaElementAudioSourceNode and have
HTMLMediaElement notify it when the flag changes. In
updateResamplerIfNeeded(), use the playback rate as a resampling
factor only when preservesPitch is false.

With 310098@main, we progressed two subtests in `preserves-pitch.html`
WPT but regressed two as well (preservesPitch=true) cases. Now, this
patch fixes them and unskip whole test altogether.

* LayoutTests/TestExpectations: Unskip (if unskipped, it would have caught the issue before)
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/preserves-pitch-expected.txt:
* Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:
(WebCore::MediaElementAudioSourceNode::setPreservesPitch):
(WebCore::MediaElementAudioSourceNode::updateResamplerIfNeeded):
* Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.h:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setPreservesPitch):
@Ahmad-S792 Ahmad-S792 requested review from cdumez and rniwa as code owners March 28, 2026 04:47
@Ahmad-S792 Ahmad-S792 self-assigned this Mar 28, 2026
@Ahmad-S792 Ahmad-S792 added the Media Bugs related to the HTML 5 Media elements. label Mar 28, 2026
@Ahmad-S792
Copy link
Copy Markdown
Contributor Author

This is flaky. :-(

@Ahmad-S792 Ahmad-S792 marked this pull request as draft March 28, 2026 05:12
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 28, 2026
@Ahmad-S792 Ahmad-S792 closed this Mar 30, 2026
@Ahmad-S792 Ahmad-S792 deleted the eng/MediaElementAudioSourceNode-should-account-for-preservesPitch-when-resampling-for-playback-rate branch April 10, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Media Bugs related to the HTML 5 Media elements. merging-blocked Applied to prevent a change from being merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants