| Summary: | REGRESSION(r290175): Texture upload from video and user media is slower than expected for non-GPUP WebGL | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kimmo Kinnunen <kkinnunen> | ||||||||
| Component: | WebGL | Assignee: | Kimmo Kinnunen <kkinnunen> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | dino, eric.carlson, ews-watchlist, glenn, jer.noble, kbr, kkinnunen, philipj, sergio, webkit-bug-importer, youennf | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Kimmo Kinnunen
2022-02-22 05:51:09 PST
Created attachment 452857 [details]
Patch
Comment on attachment 452857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452857&action=review > Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:123 > + auto result = m_connection->sendSync(Messages::RemoteVideoFrameObjectHeap::PixelBuffer(read()), Messages::RemoteVideoFrameObjectHeap::PixelBuffer::Reply(pixelBuffer), 0, defaultTimeout); I would tend to move that code in RemoteVideoFrameObjectHeapProxy instead of here. At some point, I think we should also go through RemoteVideoFrameObjectHeapProxy to release the video frame. Then RemoteVideoFrameProxy would not need to have a m_connection, nor do any direct IPC, the heap proxy would do it. > Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.h:97 > + static inline Seconds defaultTimeout = 10_s; 10_s seems like a lot of time. It probably does not matter right now since all our frames are created in GPUProcess so value could be 0_s. Comment on attachment 452857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452857&action=review > Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:125 > + return nullptr; If there is no result, we should probably create a black pixel buffer, so that libwebrtc is made happy. Comment on attachment 452857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452857&action=review thanks! >> Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:123 >> + auto result = m_connection->sendSync(Messages::RemoteVideoFrameObjectHeap::PixelBuffer(read()), Messages::RemoteVideoFrameObjectHeap::PixelBuffer::Reply(pixelBuffer), 0, defaultTimeout); > > I would tend to move that code in RemoteVideoFrameObjectHeapProxy instead of here. > At some point, I think we should also go through RemoteVideoFrameObjectHeapProxy to release the video frame. > Then RemoteVideoFrameProxy would not need to have a m_connection, nor do any direct IPC, the heap proxy would do it. Talked offline. Currently it's one message, so it doesn't need state stored in proxy. The current functionality of the proxy itself should be removed once the IPC system is improved to not require such duplicated code for sending big blocks of memory. >> Source/WebKit/WebProcess/GPU/media/RemoteVideoFrameProxy.cpp:125 >> + return nullptr; > > If there is no result, we should probably create a black pixel buffer, so that libwebrtc is made happy. done Created attachment 452950 [details]
Patch for landing
Created attachment 452954 [details]
Patch for landing
Committed r290413 (247721@main): <https://commits.webkit.org/247721@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452954 [details]. |