Bug 221696 – [GPUP] 2 web audio WPT tests fail when media in GPU Process is enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Peng Liu]

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

[Radar WebKit Bug Importer]

<rdar://problem/74440772>

[Peng Liu]

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]

Committed r274493 (235349@main): <https://commits.webkit.org/235349@main>

[Jer Noble]

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]

(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]

(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]

Created attachment 423837 [details]
Patch

[Peng Liu]

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 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]

(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]

Created attachment 423892 [details]
Patch for landing

[Jer Noble]

Created attachment 423900 [details]
Patch for landing

[Jer Noble]

Still waiting on EWS bots to run tests before landing.

[EWS]

Committed r274891: <https://commits.webkit.org/r274891>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423900 [details].