Bug 236502 - WebGL GPUP: Crash when running fast/mediastream/getUserMedia-to-canvas-1.html
Summary: WebGL GPUP: Crash when running fast/mediastream/getUserMedia-to-canvas-1.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on: 236701
Blocks:
  Show dependency treegraph
 
Reported: 2022-02-11 06:44 PST by Kimmo Kinnunen
Modified: 2022-02-21 06:07 PST (History)
16 users (show)

See Also:


Attachments
Patch (38.51 KB, patch)
2022-02-17 07:43 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (61.57 KB, patch)
2022-02-18 04:27 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (62.68 KB, patch)
2022-02-18 04:56 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (63.16 KB, patch)
2022-02-18 06:49 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (63.61 KB, patch)
2022-02-18 09:05 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (63.83 KB, patch)
2022-02-18 10:31 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
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-11 06:44:32 PST
WebGL GPUP: Crash when running fast/mediastream/getUserMedia-to-canvas-1.html 


Needs implementation of user media capture'd media player paint to canvas
Comment 1 Radar WebKit Bug Importer 2022-02-12 19:46:11 PST
<rdar://problem/88862921>
Comment 2 Kimmo Kinnunen 2022-02-17 07:43:59 PST
Created attachment 452365 [details]
Patch
Comment 3 Simon Fraser (smfr) 2022-02-17 10:51:32 PST
Comment on attachment 452365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452365&action=review

> Source/WebCore/platform/VideoFrame.h:56
>      virtual bool isCV() const { return false; }

The CV abbreviation is a bit obscure here.

> Source/WebCore/platform/VideoFrame.h:57
> +    virtual RefPtr<WebCore::VideoFrameCV> asVideoFrameCV() = 0;

This seems like a layering violation. Normally we'd do this by adding SPECIALIZE_TYPE_TRAITS macros to allow for is<> and downcast<>

> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGLCocoa.cpp:62
> +        videoFrameCV = &downcast<WebCore::VideoFrameCV>(*videoFrame);

This doesn't use asVideoFrameCV()?
Comment 4 Kimmo Kinnunen 2022-02-17 11:08:58 PST
Comment on attachment 452365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452365&action=review

>> Source/WebCore/platform/VideoFrame.h:57
>> +    virtual RefPtr<WebCore::VideoFrameCV> asVideoFrameCV() = 0;
> 
> This seems like a layering violation. Normally we'd do this by adding SPECIALIZE_TYPE_TRAITS macros to allow for is<> and downcast<>

It has those. This method is different. It converts to the type.
This is the polymorphic conversion function.

>> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGLCocoa.cpp:62
>> +        videoFrameCV = &downcast<WebCore::VideoFrameCV>(*videoFrame);
> 
> This doesn't use asVideoFrameCV()?

No, that's a different function.
Comment 5 Eric Carlson 2022-02-17 11:17:25 PST
Comment on attachment 452365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452365&action=review

>> Source/WebCore/platform/VideoFrame.h:56
>>      virtual bool isCV() const { return false; }
> 
> The CV abbreviation is a bit obscure here.

It seems fine to me, and it won't be the first time we've used it, e.g. GraphicsContextGLCV, PixelBufferConformerCV, GraphicsContextGLCVCocoa
Comment 6 Eric Carlson 2022-02-17 11:17:44 PST
r=me once the bots are happy
Comment 7 Kimmo Kinnunen 2022-02-18 04:27:53 PST
Created attachment 452506 [details]
Patch
Comment 8 Kimmo Kinnunen 2022-02-18 04:56:08 PST
Created attachment 452509 [details]
Patch
Comment 9 Kimmo Kinnunen 2022-02-18 06:49:17 PST
Created attachment 452515 [details]
Patch for landing
Comment 10 Kimmo Kinnunen 2022-02-18 09:05:21 PST
Created attachment 452524 [details]
Patch for landing
Comment 11 Kimmo Kinnunen 2022-02-18 10:31:09 PST
Created attachment 452538 [details]
Patch for landing
Comment 12 EWS 2022-02-18 14:49:26 PST
Committed r290175 (247506@main): <https://commits.webkit.org/247506@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452538 [details].
Comment 13 Kimmo Kinnunen 2022-02-18 22:37:15 PST
nongpup webgl started to assert, will fix in bug 236878
Comment 14 youenn fablet 2022-02-21 06:07:50 PST
Comment on attachment 452538 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=452538&action=review

> Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.cpp:190
> +    if (!videoFrame || !is<RemoteVideoFrameProxy>(*videoFrame))

This should probably be a FIXME, painting VP8 WebRTC streamed content on WebGL may exercise that code path.
I filed https://bugs.webkit.org/show_bug.cgi?id=236972 to track this.