| Summary: | [GPUProcess] Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
| Component: | Web Audio | Assignee: | Chris Dumez <cdumez> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | calvaris, cdumez, darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, peng.liu6, philipj, pnormand, sam, sergio, vjaquez, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 212611, 217606 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Dumez
2020-10-27 11:55:41 PDT
*** Bug 217990 has been marked as a duplicate of this bug. *** Created attachment 412454 [details]
Patch
Comment on attachment 412454 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412454&action=review > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:132 > + completionHandler(Exception { InvalidStateError, "Failed to start the audio device"_s }); Nit. Can be done in one line as in DefaultAudioDestinationNode::resume(). > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:148 > + completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start audio device"_s })); Nit. Maybe we can use the same error message as in DefaultAudioDestinationNode::startRendering()? s/"Failed to start audio device"_s/"Failed to start the audio device"_s/ > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:163 > + completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop audio device"_s })); Ditto. s/"Failed to stop audio device"_s/"Failed to stop the audio device"_s/ > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:106 > + auto success = !m_audioOutputUnitAdaptor.start(); Nit. OK as it is. Maybe "auto success = m_audioOutputUnitAdaptor.start() == noErr" is better? > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:118 > + auto success = !m_audioOutputUnitAdaptor.stop(); Ditto. (In reply to Peng Liu from comment #3) > Comment on attachment 412454 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412454&action=review > > > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:132 > > + completionHandler(Exception { InvalidStateError, "Failed to start the audio device"_s }); > > Nit. Can be done in one line as in DefaultAudioDestinationNode::resume(). > > > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:148 > > + completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start audio device"_s })); > > Nit. Maybe we can use the same error message as in > DefaultAudioDestinationNode::startRendering()? > s/"Failed to start audio device"_s/"Failed to start the audio device"_s/ > > > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:163 > > + completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop audio device"_s })); > > Ditto. s/"Failed to stop audio device"_s/"Failed to stop the audio device"_s/ > > > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:106 > > + auto success = !m_audioOutputUnitAdaptor.start(); > > Nit. OK as it is. Maybe "auto success = m_audioOutputUnitAdaptor.start() == > noErr" is better? > > > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:118 > > + auto success = !m_audioOutputUnitAdaptor.stop(); > > Ditto. Thanks for looking. I will fix all these shortly. Created attachment 412460 [details]
Patch
Comment on attachment 412460 [details]
Patch
r=me
Committed r269073: <https://trac.webkit.org/changeset/269073> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412460 [details]. |