Bug 221696

Summary: [GPUP] 2 web audio WPT tests fail when media in GPU Process is enabled
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: Web AudioAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, eric.carlson, ews-watchlist, glenn, jer.noble, jonlee, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 221830    
Attachments:
Description Flags
Patch
eric.carlson: review+
Patch for landing
ews-feeder: commit-queue-
Patch for landing none

Peng Liu
Reported 2021-02-10 10:53:30 PST
imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https.html imported/w3c/web-platform-tests/webaudio/the-audio-api/the-destinationnode-interface/destination.html imported/w3c/web-platform-tests/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/no-cors.https.html
Attachments
Patch (6.08 KB, patch)
2021-03-21 14:23 PDT, Jer Noble 		eric.carlson: review+
DetailsFormatted DiffDiff
Patch for landing (4.72 KB, patch)
2021-03-22 08:56 PDT, Jer Noble 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch for landing (6.75 KB, patch)
2021-03-22 09:46 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2021-02-17 10:54:13 PST
<rdar://problem/74440772>
Peng Liu
Comment 2 2021-03-16 10:52:16 PDT
The failure of imported/w3c/web-platform-tests/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/no-cors.https.html is tracked in platform/mac-wk1/TestExpectations. It is not a regression due to "Media in GPU Process".
Peng Liu
Comment 3 2021-03-16 10:55:45 PDT
Committed r274493 (235349@main): <https://commits.webkit.org/235349@main>
Jer Noble
Comment 4 2021-03-19 09:21:05 PDT
This test no longer appears to be failing: imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/active-processing.https.html See: https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode-interface%2Factive-processing.https.html
Peng Liu
Comment 5 2021-03-19 14:47:04 PDT
(In reply to Jer Noble from comment #4) > This test no longer appears to be failing: > imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode- > interface/active-processing.https.html > > See: > https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb- > platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-convolvernode- > interface%2Factive-processing.https.html Seems flaky now...
Jer Noble
Comment 6 2021-03-21 14:02:50 PDT
(In reply to Peng Liu from comment #5) > Seems flaky now... Yes, but it's flaky on the WK1 bots, so this is likely not a GPU process issue.
Jer Noble
Comment 7 2021-03-21 14:23:49 PDT
Created attachment 423837 [details] Patch
Peng Liu
Comment 8 2021-03-21 16:16:14 PDT
Comment on attachment 423837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423837&action=review > Source/WebKit/ChangeLog:9 > + Test fails when AudioDestination.maxChannelCount is queried and returns 0; pipe the correct Nice! I was trying to fix this bug by making `AudioDestination::hardwareSampleRate()` and `AudioDestination::maxChannelCount()` non-static. This patch looks much simpler!
Chris Dumez
Comment 9 2021-03-21 21:35:29 PDT
Comment on attachment 423837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423837&action=review > Source/WebKit/WebProcess/GPU/media/RemoteAudioSession.h:73 > + size_t maximumNumberOfOutputChannels() const final { return m_configuration.maximumNumberOfOutputChannels; } Here it says size_t > Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.h:42 > + uint64_t maximumNumberOfOutputChannels { 0 }; Not a big deal but I don’t see why we are using uint64_t here but size_t elsewhere. Why aren’t we consistent?
Chris Dumez
Comment 10 2021-03-21 21:37:39 PDT
(In reply to Chris Dumez from comment #9) > Comment on attachment 423837 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423837&action=review > > > Source/WebKit/WebProcess/GPU/media/RemoteAudioSession.h:73 > > + size_t maximumNumberOfOutputChannels() const final { return m_configuration.maximumNumberOfOutputChannels; } > > Here it says size_t > > > Source/WebKit/WebProcess/GPU/media/RemoteAudioSessionConfiguration.h:42 > > + uint64_t maximumNumberOfOutputChannels { 0 }; > > Not a big deal but I don’t see why we are using uint64_t here but size_t > elsewhere. Why aren’t we consistent? Note that we used to support a 32bit process IPCing a 64bit process but this has not been the case in a while. Therefore, sending a size_t over IPC should be fine afaik.
Jer Noble
Comment 11 2021-03-22 08:56:18 PDT
Created attachment 423892 [details] Patch for landing
Jer Noble
Comment 12 2021-03-22 09:46:52 PDT
Created attachment 423900 [details] Patch for landing
Jer Noble
Comment 13 2021-03-22 12:20:52 PDT
Still waiting on EWS bots to run tests before landing.
EWS
Comment 14 2021-03-23 12:21:12 PDT
Committed r274891: <https://commits.webkit.org/r274891> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423900 [details].
Note You need to log in before you can comment on or make changes to this bug.