Bug 237034 - REGRESSION(r290175): Texture upload from video and user media is slower than expected for non-GPUP WebGL
Summary: REGRESSION(r290175): Texture upload from video and user media is slower than ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-22 05:51 PST by Kimmo Kinnunen
Modified: 2022-02-23 22:38 PST (History)
11 users (show)

See Also:


Attachments
Patch (7.67 KB, patch)
2022-02-22 05:53 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch for landing (7.65 KB, patch)
2022-02-23 01:12 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch for landing (7.85 KB, patch)
2022-02-23 02:23 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2022-02-22 05:51:09 PST
REGRESSION(r290175): Texture upload from video and user media is slower than expected for non-GPUP WebGL
Comment 1 Kimmo Kinnunen 2022-02-22 05:53:14 PST
Created attachment 452857 [details]
Patch
Comment 2 youenn fablet 2022-02-22 06:09:23 PST
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 3 youenn fablet 2022-02-22 06:13:25 PST
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 4 Kimmo Kinnunen 2022-02-23 00:52:19 PST
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
Comment 5 Kimmo Kinnunen 2022-02-23 01:12:41 PST
Created attachment 452950 [details]
Patch for landing
Comment 6 Kimmo Kinnunen 2022-02-23 02:23:04 PST
Created attachment 452954 [details]
Patch for landing
Comment 7 EWS 2022-02-23 22:37:08 PST
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].
Comment 8 Radar WebKit Bug Importer 2022-02-23 22:38:21 PST
<rdar://problem/89400347>