Bug 206286 – Add support for MediaStream video track rendering in GPUProcess

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

[youenn fablet]

Add support for MediaStream video track rendering in GPUProcess

[youenn fablet]

Created attachment 387783 [details]
Patch

[youenn fablet]

Rendering does not work 100%, video frame is only updated when a full refresh is needed, like a window resize.

[Eric Carlson]

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

[youenn fablet]

Created attachment 387900 [details]
Patch

[youenn fablet]

Created attachment 387901 [details]
Patch

[youenn fablet]

Created attachment 387910 [details]
Patch

[Eric Carlson]

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.

[youenn fablet]

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

[youenn fablet]

Created attachment 387921 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 387921 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 387921 [details]
Patch for landing

Clearing flags on attachment: 387921

Committed r254688: <https://trac.webkit.org/changeset/254688>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58647949>

[Matt Lewis]

Reverted r254688 for reason:

This caused internal test crashes.

Committed r254778: <https://trac.webkit.org/changeset/254778>

[youenn fablet]

Created attachment 388216 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 388216 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 388216 [details]
Patch for landing

Clearing flags on attachment: 388216

Committed r254817: <https://trac.webkit.org/changeset/254817>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.