Skip to content

Conversation

@jernoble
Copy link
Contributor

@jernoble jernoble commented Feb 26, 2025

d5fefcb

[Cocoa] Add support for per-media-element soundStageSize
rdar://145660143
https://bugs.webkit.org/show_bug.cgi?id=288601

Reviewed by Andy Estes.

Previously, when web content entered fullscreen modes, the increased soundStageSize
would propagate down to the AVAudioSession, where that size was applied to all audio
generated by every web page. With CASpatialAudioExperience, the soundStageSize can
be applied granularly per audio-generating object.

Drive-by fix: address a review comment made against 291033@main and set NS_ASSUME_NONNULL
in AudioToolboxCoreSPI.h.

* Source/WebCore/PAL/pal/spi/cocoa/AudioToolboxCoreSPI.h:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setSoundStageSize):
* Source/WebCore/html/HTMLMediaElement.h:
(WebCore::HTMLMediaElement::soundStageSize const):
* Source/WebCore/platform/audio/cocoa/SpatialAudioExperienceHelper.h:
* Source/WebCore/platform/audio/cocoa/SpatialAudioExperienceHelper.mm:
(WebCore::toCASoundStageSize):
(WebCore::createSpatialAudioExperienceWithOptions):
(WebCore::createExperienceWithOptions): Deleted.
* Source/WebCore/platform/graphics/MediaPlayer.cpp:
(WebCore::MediaPlayer::soundStageSize const):
(WebCore::MediaPlayer::soundStageSizeDidChange):
* Source/WebCore/platform/graphics/MediaPlayer.h:
(WebCore::MediaPlayerClient::mediaPlayerSoundStageSize const):
* Source/WebCore/platform/graphics/MediaPlayerEnums.h:
* Source/WebCore/platform/graphics/MediaPlayerPrivate.h:
(WebCore::MediaPlayerPrivateInterface::soundStageSizeDidChange):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::updateSpatialTrackingLabel):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::updateSpatialTrackingLabel):
* Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm:
(WebCore::MediaPlayerPrivateWebM::updateSpatialTrackingLabel):
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerSoundStageSize const):
(WebKit::RemoteMediaPlayerProxy::setSoundStageSize):
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in:
* Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerRenderingModeChanged):
(WebKit::RemoteMediaPlayerProxy::mediaPlayerOnNewVideoFrameMetadata):
(WebKit::RemoteMediaPlayerProxy::nativeImageForCurrentTime):
(WebKit::RemoteMediaPlayerProxy::colorSpace):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::soundStageSizeDidChange):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:

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

48b7c72

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@jernoble jernoble self-assigned this Feb 26, 2025
@jernoble jernoble added the Media Bugs related to the HTML 5 Media elements. label Feb 26, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@jernoble jernoble force-pushed the eng/Cocoa-Add-support-for-per-media-element-soundStageSize branch from d698656 to f342eb5 Compare February 26, 2025 18:57
@jernoble jernoble force-pushed the eng/Cocoa-Add-support-for-per-media-element-soundStageSize branch from f342eb5 to e508c79 Compare February 26, 2025 19:01
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@jernoble jernoble force-pushed the eng/Cocoa-Add-support-for-per-media-element-soundStageSize branch from e508c79 to 48b7c72 Compare February 26, 2025 20:11
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Feb 26, 2025
@jernoble jernoble requested a review from a team February 26, 2025 20:13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a little safer to write this as a switch statement that RELEASE_ASSERT_NOT_REACHED()s in the default case. Right now this implementation asserts that the known enum values numerically match, but will still allow you to cast an unknown value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll address this in a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SoundStageSize mediaPlayerSoundStageSize() const final { return m_soundStageSize; }
SoundStageSize mediaPlayerSoundStageSize() const final { return m_soundStageSize; }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change still needed since you added a using above?

@jernoble jernoble added the merge-queue Applied to send a pull request to merge-queue label Feb 27, 2025
rdar://145660143
https://bugs.webkit.org/show_bug.cgi?id=288601

Reviewed by Andy Estes.

Previously, when web content entered fullscreen modes, the increased soundStageSize
would propagate down to the AVAudioSession, where that size was applied to all audio
generated by every web page. With CASpatialAudioExperience, the soundStageSize can
be applied granularly per audio-generating object.

Drive-by fix: address a review comment made against 291033@main and set NS_ASSUME_NONNULL
in AudioToolboxCoreSPI.h.

* Source/WebCore/PAL/pal/spi/cocoa/AudioToolboxCoreSPI.h:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setSoundStageSize):
* Source/WebCore/html/HTMLMediaElement.h:
(WebCore::HTMLMediaElement::soundStageSize const):
* Source/WebCore/platform/audio/cocoa/SpatialAudioExperienceHelper.h:
* Source/WebCore/platform/audio/cocoa/SpatialAudioExperienceHelper.mm:
(WebCore::toCASoundStageSize):
(WebCore::createSpatialAudioExperienceWithOptions):
(WebCore::createExperienceWithOptions): Deleted.
* Source/WebCore/platform/graphics/MediaPlayer.cpp:
(WebCore::MediaPlayer::soundStageSize const):
(WebCore::MediaPlayer::soundStageSizeDidChange):
* Source/WebCore/platform/graphics/MediaPlayer.h:
(WebCore::MediaPlayerClient::mediaPlayerSoundStageSize const):
* Source/WebCore/platform/graphics/MediaPlayerEnums.h:
* Source/WebCore/platform/graphics/MediaPlayerPrivate.h:
(WebCore::MediaPlayerPrivateInterface::soundStageSizeDidChange):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::updateSpatialTrackingLabel):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::updateSpatialTrackingLabel):
* Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm:
(WebCore::MediaPlayerPrivateWebM::updateSpatialTrackingLabel):
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerSoundStageSize const):
(WebKit::RemoteMediaPlayerProxy::setSoundStageSize):
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:
* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in:
* Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerProxyCocoa.mm:
(WebKit::RemoteMediaPlayerProxy::mediaPlayerRenderingModeChanged):
(WebKit::RemoteMediaPlayerProxy::mediaPlayerOnNewVideoFrameMetadata):
(WebKit::RemoteMediaPlayerProxy::nativeImageForCurrentTime):
(WebKit::RemoteMediaPlayerProxy::colorSpace):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::soundStageSizeDidChange):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:

Canonical link: https://commits.webkit.org/291268@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Cocoa-Add-support-for-per-media-element-soundStageSize branch from 48b7c72 to d5fefcb Compare February 27, 2025 23:41
@webkit-commit-queue
Copy link
Collaborator

Committed 291268@main (d5fefcb): https://commits.webkit.org/291268@main

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

@webkit-commit-queue webkit-commit-queue merged commit d5fefcb into WebKit:main Feb 27, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 27, 2025
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants