Bug 205922 – RemoteVideoSample should be able to support canvas video samples

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

RemoteVideoSample should be able to support canvas video samples

[youenn fablet]

Created attachment 387100 [details]
Patch

[Eric Carlson]

Comment on attachment 387100 [details]
Patch

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

> Source/WebCore/platform/graphics/RemoteVideoSample.cpp:56
> +    auto ioSurface =  IOSurface::create(size, sRGBColorSpaceRef(), IOSurface::Format::RGBA);

Minor nit: you might as well move this down a few lines in case CVPixelBufferLockBaseAddress fails.

> Source/WebCore/platform/graphics/RemoteVideoSample.cpp:109
> +            RELEASE_LOG_ERROR(Media, "RemoteVideoSample::create: CVPixelBufferGetIOSurface returned nullptr");

This log message is now incorrect.

> LayoutTests/http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable-gpuprocess-expected.txt:5
> +Harness Error (TIMEOUT), message = null
> +
> +TIMEOUT Verify MediaRecorder is working in GPUProcess Test timed out

Maybe add a comment to the ChangeLog about why the new test fails?

[youenn fablet]

(In reply to Eric Carlson from comment #2)
> Comment on attachment 387100 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387100&action=review
> 
> > Source/WebCore/platform/graphics/RemoteVideoSample.cpp:56
> > +    auto ioSurface =  IOSurface::create(size, sRGBColorSpaceRef(), IOSurface::Format::RGBA);
> 
> Minor nit: you might as well move this down a few lines in case
> CVPixelBufferLockBaseAddress fails.

OK

> > Source/WebCore/platform/graphics/RemoteVideoSample.cpp:109
> > +            RELEASE_LOG_ERROR(Media, "RemoteVideoSample::create: CVPixelBufferGetIOSurface returned nullptr");
> 
> This log message is now incorrect.

Updated.

> > LayoutTests/http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable-gpuprocess-expected.txt:5
> > +Harness Error (TIMEOUT), message = null
> > +
> > +TIMEOUT Verify MediaRecorder is working in GPUProcess Test timed out
> 
> Maybe add a comment to the ChangeLog about why the new test fails?

Last minute change to the test made it fail, it is missing a t.done.
Will fix it.

[youenn fablet]

Created attachment 387200 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 387200 [details]
Patch for landing

Clearing flags on attachment: 387200

Committed r254256: <https://trac.webkit.org/changeset/254256>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58442026>

[Truitt Savell]

The new test http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable-gpuprocess.html 

added in https://trac.webkit.org/changeset/254256/webkit

is a flaky failure on Mojave wk2

Tracking in https://bugs.webkit.org/show_bug.cgi?id=206437