Skip to content

[WebAudio] Implement AudioContext::outputLatency#38923

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
jyavenard:eng/WebAudio-Implement-AudioContext-outputLatency
Jan 15, 2025
Merged

[WebAudio] Implement AudioContext::outputLatency#38923
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
jyavenard:eng/WebAudio-Implement-AudioContext-outputLatency

Conversation

@jyavenard

@jyavenard jyavenard commented Jan 13, 2025

Copy link
Copy Markdown
Member

420f89a

[WebAudio] Implement AudioContext::outputLatency
https://bugs.webkit.org/show_bug.cgi?id=285826
rdar://142794341

Reviewed by Youenn Fablet.

Add implementation for Cocoa platforms, other platforms will return 0 for now.

* LayoutTests/imported/w3c/web-platform-tests/webaudio/idlharness.https.window-expected.txt:
* Source/WebCore/Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::outputLatency):
* Source/WebCore/Modules/webaudio/AudioContext.h:
* Source/WebCore/Modules/webaudio/AudioContext.idl:
* Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:
(WebCore::DefaultAudioDestinationNode::outputLatency const):
* Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h:
* Source/WebCore/platform/audio/AudioDestination.h:
(WebCore::AudioDestination::outputLatency const):
* Source/WebCore/platform/audio/AudioSession.h:
* Source/WebCore/platform/audio/SharedAudioDestination.cpp:
(WebCore::SharedAudioDestinationAdapter::outputLatency const):
(WebCore::SharedAudioDestinationAdapter::protectedDestination const):
(WebCore::SharedAudioDestination::outputLatency const):
(WebCore::SharedAudioDestination::protectedOutputAdapter const):
(WebCore::SharedAudioDestinationAdapter::protectedDestination): Deleted.
(WebCore::SharedAudioDestination::protectedOutputAdapter): Deleted.
* Source/WebCore/platform/audio/SharedAudioDestination.h:
* Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:
(WebCore::AudioDestinationCocoa::outputLatency const):
* Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h:
* Source/WebCore/platform/audio/cocoa/AudioOutputUnitAdaptor.cpp:
(WebCore::AudioOutputUnitAdaptor::outputLatency const):
* Source/WebCore/platform/audio/cocoa/AudioOutputUnitAdaptor.h:
* Source/WebCore/platform/audio/ios/AudioSessionIOS.h:
* Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:
(WebCore::AudioSessionIOS::outputLatency const):
* Source/WebCore/platform/audio/mac/AudioSessionMac.h:
* Source/WebCore/platform/audio/mac/AudioSessionMac.mm:
(WebCore::AudioSessionMac::outputLatency const):
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp:
(WebKit::RemoteAudioDestinationManager::createAudioDestination):
(WebKit::RemoteAudioDestinationManager::startAudioDestination):
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.h:
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.messages.in:
* Source/WebKit/GPUProcess/media/RemoteAudioSessionProxy.cpp:
(WebKit::RemoteAudioSessionProxy::configuration):
* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::connection):
(WebKit::RemoteAudioDestinationProxy::startRendering):
(WebKit::RemoteAudioDestinationProxy::outputLatency const):
* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSession.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.serialization.in:

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

a575fca

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
✅ 🧪 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

@jyavenard jyavenard requested a review from cdumez as a code owner January 13, 2025 10:07
@jyavenard jyavenard self-assigned this Jan 13, 2025
@jyavenard jyavenard added the Web Audio Bugs against the Web Audio API label Jan 13, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 13, 2025
@jyavenard jyavenard removed the merging-blocked Applied to prevent a change from being merged label Jan 13, 2025
@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from 508e4ab to 0ab4a63 Compare January 13, 2025 10:31
@jyavenard jyavenard requested review from jernoble and youennf January 13, 2025 10:32
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 13, 2025
@jyavenard jyavenard removed the merging-blocked Applied to prevent a change from being merged label Jan 14, 2025
@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from 0ab4a63 to c5d7ea4 Compare January 14, 2025 00:42
@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from c5d7ea4 to f2267bb Compare January 14, 2025 00:44

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you could go down this codepath in both cases (Minimal and Enhanced)

Suggested change
if (noiseInjectionPolicies().contains(NoiseInjectionPolicy::Minimal))
if (noiseInjectionPolicies())

@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from f2267bb to 088fb33 Compare January 14, 2025 01:53
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be better to leave destination do this computation?
That way, we can do the computation in GPUProcess and remove the need to send to WebProcess the audio session latency.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Doing so would require to have a way for the RemoteAudioDestination/RemoteAudioDestinationProxy be notified when the AudioSession changes. It's a much bigger change.

And currently the RemoteAudioSession in the WebContent is being notified of changes of AudioSession in the GPU process. So I re-used that.

it made the change much smaller.

I could move that arithmetic in the AudioDestination, but then it kind of blur which latency are we talking about. The AudioDestination itself, the AudioSession, both?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer we would move the computation to the AudioDestination.
It is probably the audio destination that will handle setSinkId computation as well, so that seems better this way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would tend to move this computation to AudioOuptutUnitAdaptor.
The reason is that AudioContext can be set to a particular device via setSinkId.

To handle the case of audio context playing on the device device and default device is changing, AudioOuptutUnitAdaptor would ideally be notified that the default device is changing and would do the recomputation if it is not set to a particular device via setSinkId.
And then send the updated value to its AudioOuptutUnitAdaptor client.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't currently support setSinkId on the AudioContext so I will take this as a follow-up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also don't see how we can retrieve the device's latency from the AudioUnit

@youennf youennf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r=me.
I would tend to have AudioDestination be the one to do the latency computation.
This eases handling the case of not playing context as well as future setSinkId case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems odd to return the audiosession latency even if AudioContext is not playing.
I'll tend to move the logic to add the latencies in destination.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Limited it to when the context is playing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer we would move the computation to the AudioDestination.
It is probably the audio destination that will handle setSinkId computation as well, so that seems better this way.

@jyavenard jyavenard removed the merging-blocked Applied to prevent a change from being merged label Jan 14, 2025
@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from 088fb33 to 6b74781 Compare January 14, 2025 23:00
@jyavenard jyavenard force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from 6b74781 to a575fca Compare January 14, 2025 23:59
@jyavenard jyavenard added the merge-queue Applied to send a pull request to merge-queue label Jan 15, 2025
https://bugs.webkit.org/show_bug.cgi?id=285826
rdar://142794341

Reviewed by Youenn Fablet.

Add implementation for Cocoa platforms, other platforms will return 0 for now.

* LayoutTests/imported/w3c/web-platform-tests/webaudio/idlharness.https.window-expected.txt:
* Source/WebCore/Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::outputLatency):
* Source/WebCore/Modules/webaudio/AudioContext.h:
* Source/WebCore/Modules/webaudio/AudioContext.idl:
* Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:
(WebCore::DefaultAudioDestinationNode::outputLatency const):
* Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.h:
* Source/WebCore/platform/audio/AudioDestination.h:
(WebCore::AudioDestination::outputLatency const):
* Source/WebCore/platform/audio/AudioSession.h:
* Source/WebCore/platform/audio/SharedAudioDestination.cpp:
(WebCore::SharedAudioDestinationAdapter::outputLatency const):
(WebCore::SharedAudioDestinationAdapter::protectedDestination const):
(WebCore::SharedAudioDestination::outputLatency const):
(WebCore::SharedAudioDestination::protectedOutputAdapter const):
(WebCore::SharedAudioDestinationAdapter::protectedDestination): Deleted.
(WebCore::SharedAudioDestination::protectedOutputAdapter): Deleted.
* Source/WebCore/platform/audio/SharedAudioDestination.h:
* Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:
(WebCore::AudioDestinationCocoa::outputLatency const):
* Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h:
* Source/WebCore/platform/audio/cocoa/AudioOutputUnitAdaptor.cpp:
(WebCore::AudioOutputUnitAdaptor::outputLatency const):
* Source/WebCore/platform/audio/cocoa/AudioOutputUnitAdaptor.h:
* Source/WebCore/platform/audio/ios/AudioSessionIOS.h:
* Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:
(WebCore::AudioSessionIOS::outputLatency const):
* Source/WebCore/platform/audio/mac/AudioSessionMac.h:
* Source/WebCore/platform/audio/mac/AudioSessionMac.mm:
(WebCore::AudioSessionMac::outputLatency const):
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp:
(WebKit::RemoteAudioDestinationManager::createAudioDestination):
(WebKit::RemoteAudioDestinationManager::startAudioDestination):
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.h:
* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.messages.in:
* Source/WebKit/GPUProcess/media/RemoteAudioSessionProxy.cpp:
(WebKit::RemoteAudioSessionProxy::configuration):
* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::connection):
(WebKit::RemoteAudioDestinationProxy::startRendering):
(WebKit::RemoteAudioDestinationProxy::outputLatency const):
* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSession.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.h:
* Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.serialization.in:

Canonical link: https://commits.webkit.org/288904@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebAudio-Implement-AudioContext-outputLatency branch from a575fca to 420f89a Compare January 15, 2025 00:57
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 288904@main (420f89a): https://commits.webkit.org/288904@main

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

@webkit-commit-queue webkit-commit-queue merged commit 420f89a into WebKit:main Jan 15, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Web Audio Bugs against the Web Audio API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants