| Summary: | Implement a remote Internal Unit in GPUProcess for audio MediaStreamTrack rendering | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | annulen, cdumez, eric.carlson, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, philipj, ryuan.choi, sergio, tommyw, webkit-bug-importer, youennf | ||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=225601 https://bugs.webkit.org/show_bug.cgi?id=226095 |
||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
youenn fablet
2021-05-10 11:35:00 PDT
Created attachment 428188 [details]
Patch
Created attachment 428367 [details]
Patch
Created attachment 428369 [details]
Patch
A follow-up patch will remove the code sending individual audio track content to GPUProcess. Comment on attachment 428369 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428369&action=review > Source/WebKit/WebProcess/GPU/webrtc/AudioMediaStreamTrackRendererInternalUnitManager.cpp:107 > + for (auto proxy : m_proxies.values()) > + proxy->restartIfNeeded(); Will we want to always relaunch the GPU process as soon as it exits, e.g. if under memory pressure? Created attachment 428924 [details]
Patch
Created attachment 428929 [details]
Patch for landing
(In reply to Eric Carlson from comment #5) > Comment on attachment 428369 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=428369&action=review > > > Source/WebKit/WebProcess/GPU/webrtc/AudioMediaStreamTrackRendererInternalUnitManager.cpp:107 > > + for (auto proxy : m_proxies.values()) > > + proxy->restartIfNeeded(); > > Will we want to always relaunch the GPU process as soon as it exits, e.g. if > under memory pressure? Good point, GPU Process will only restart if playing audio. Otherwise, it will wait for the unit to start to restart GPU Process. Comment on attachment 428929 [details]
Patch for landing
iOS API test failure unrelated
Patch 428929 does not build Created attachment 429164 [details]
Patch for landing
Created attachment 429273 [details]
Fix small regression compared to previous approach
(In reply to youenn fablet from comment #13) > Created attachment 429273 [details] > Fix small regression compared to previous approach Regression was introduced by start being called for every source added. ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Created attachment 429274 [details]
Patch for landing
Committed r277852 (237994@main): <https://commits.webkit.org/237994@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429274 [details]. Comment on attachment 429274 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=429274&action=review > Source/WebCore/platform/mediastream/cocoa/AudioMediaStreamTrackRendererUnit.cpp:111 > auto locker = holdLock(m_sourcesLock); Is m_sourcesLock only protecting m_sourcesCopy and not m_sources? If so, can it be renamed? Yes, it should be renamed. (In reply to youenn fablet from comment #19) > Yes, it should be renamed. I am doing it now and adopting CheckedLock at the same time. Will cc you. (In reply to Chris Dumez from comment #20) > (In reply to youenn fablet from comment #19) > > Yes, it should be renamed. > > I am doing it now and adopting CheckedLock at the same time. Will cc you. -> https://bugs.webkit.org/show_bug.cgi?id=226095 |