Bug 206286

Summary: Add support for MediaStream video track rendering in GPUProcess
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, ews-watchlist, glenn, jer.noble, jlewis3, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 206066    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Description youenn fablet 2020-01-15 05:42:30 PST
Add support for MediaStream video track rendering in GPUProcess
Comment 1 youenn fablet 2020-01-15 05:55:02 PST
Created attachment 387783 [details]
Patch
Comment 2 youenn fablet 2020-01-15 06:29:37 PST
Rendering does not work 100%, video frame is only updated when a full refresh is needed, like a window resize.
Comment 3 Eric Carlson 2020-01-15 08:59:18 PST
Comment on attachment 387783 [details]
Patch

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

> Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.cpp:138
> +    // This is only an approximation, maybe we should use a clock as well.
> +    return m_mediaTime;

I think we will need to use a clock, or time will "stop" between samples. Maybe use the wall clock to calculate the delta between samples?

> Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.messages.in:1
> +# Copyright (C) 2019 Apple Inc. All rights reserved.

2020

> Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayerManager.messages.in:1
> +# Copyright (C) 2019 Apple Inc. All rights reserved.

Ditto

> Source/WebKit/WebProcess/GPU/webrtc/SampleBufferDisplayLayer.messages.in:1
> +# Copyright (C) 2019 Apple Inc. All rights reserved.

Ditto

> Source/WebKit/WebProcess/GPU/webrtc/SampleBufferDisplayLayerIdentifier.h:2
> + * Copyright (C) 2019 Apple Inc. All rights reserved.

Ditto
Comment 4 youenn fablet 2020-01-16 02:28:29 PST
Created attachment 387900 [details]
Patch
Comment 5 youenn fablet 2020-01-16 02:54:31 PST
Created attachment 387901 [details]
Patch
Comment 6 youenn fablet 2020-01-16 05:01:11 PST
Created attachment 387910 [details]
Patch
Comment 7 Eric Carlson 2020-01-16 07:03:09 PST
Comment on attachment 387910 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm:283
> +    CFArrayRef attachmentsArray = CMSampleBufferGetSampleAttachmentsArray(sample.platformSample().sample.cmSampleBuffer, true);
> +    for (CFIndex i = 0; i < CFArrayGetCount(attachmentsArray); ++i) {
> +        CFMutableDictionaryRef attachments = checked_cf_cast<CFMutableDictionaryRef>(CFArrayGetValueAtIndex(attachmentsArray, i));
> +        CFDictionarySetValue(attachments, kCMSampleAttachmentKey_DisplayImmediately, kCFBooleanTrue);
> +    }

You mentioned this hack in the ChangeLog, but it should have a FIXME here because we're going to need a remote display layer for MSE and it will need to honor timestamps.
Comment 8 youenn fablet 2020-01-16 07:12:23 PST
(In reply to Eric Carlson from comment #7)
> Comment on attachment 387910 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387910&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm:283
> > +    CFArrayRef attachmentsArray = CMSampleBufferGetSampleAttachmentsArray(sample.platformSample().sample.cmSampleBuffer, true);
> > +    for (CFIndex i = 0; i < CFArrayGetCount(attachmentsArray); ++i) {
> > +        CFMutableDictionaryRef attachments = checked_cf_cast<CFMutableDictionaryRef>(CFArrayGetValueAtIndex(attachmentsArray, i));
> > +        CFDictionarySetValue(attachments, kCMSampleAttachmentKey_DisplayImmediately, kCFBooleanTrue);
> > +    }
> 
> You mentioned this hack in the ChangeLog, but it should have a FIXME here
> because we're going to need a remote display layer for MSE and it will need
> to honor timestamps.

OK, let's move it out of LocalSampleBufferDisplayLayer.mm for easier reuse.

I don't think this will apply to MSE though since presentation timing is key there.
MSE will probably not use LocalSampleBufferDisplayLayer.
In WebRTC, we are missing this information and are relying on attachment instead.
Comment 9 youenn fablet 2020-01-16 08:15:48 PST
Created attachment 387921 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2020-01-16 10:08:26 PST
The commit-queue encountered the following flaky tests while processing attachment 387921 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2020-01-16 10:09:15 PST
Comment on attachment 387921 [details]
Patch for landing

Clearing flags on attachment: 387921

Committed r254688: <https://trac.webkit.org/changeset/254688>
Comment 12 WebKit Commit Bot 2020-01-16 10:09:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2020-01-16 10:10:16 PST
<rdar://problem/58647949>
Comment 14 Matt Lewis 2020-01-17 16:40:23 PST
Reverted r254688 for reason:

This caused internal test crashes.

Committed r254778: <https://trac.webkit.org/changeset/254778>
Comment 15 youenn fablet 2020-01-20 00:50:49 PST
Created attachment 388216 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2020-01-20 02:38:13 PST
The commit-queue encountered the following flaky tests while processing attachment 388216 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 17 WebKit Commit Bot 2020-01-20 02:39:05 PST
Comment on attachment 388216 [details]
Patch for landing

Clearing flags on attachment: 388216

Committed r254817: <https://trac.webkit.org/changeset/254817>
Comment 18 WebKit Commit Bot 2020-01-20 02:39:07 PST
All reviewed patches have been landed.  Closing bug.