WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
221696
[GPUP] 2 web audio WPT tests fail when media in GPU Process is enabled
https://bugs.webkit.org/show_bug.cgi?id=221696
Summary
[GPUP] 2 web audio WPT tests fail when media in GPU Process is enabled
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+
Details
Formatted Diff
Diff
Patch for landing
(4.72 KB, patch)
2021-03-22 08:56 PDT
,
Jer Noble
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch for landing
(6.75 KB, patch)
2021-03-22 09:46 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug