Bug 237034 – REGRESSION(r290175): Texture upload from video and user media is slower than expected for non-GPUP WebGL

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Kimmo Kinnunen]

REGRESSION(r290175): Texture upload from video and user media is slower than expected for non-GPUP WebGL

[Kimmo Kinnunen]

Created attachment 452857 [details]
Patch

[youenn fablet]

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.

[youenn fablet]

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.

[Kimmo Kinnunen]

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

[Kimmo Kinnunen]

Created attachment 452950 [details]
Patch for landing

[Kimmo Kinnunen]

Created attachment 452954 [details]
Patch for landing

[EWS]

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

[Radar WebKit Bug Importer]

<rdar://problem/89400347>