Summary: | Add support for MediaStream video track rendering in GPUProcess | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||
Component: | WebRTC | Assignee: | 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
youenn fablet
2020-01-15 05:42:30 PST
Created attachment 387783 [details]
Patch
Rendering does not work 100%, video frame is only updated when a full refresh is needed, like a window resize. 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 Created attachment 387900 [details]
Patch
Created attachment 387901 [details]
Patch
Created attachment 387910 [details]
Patch
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. (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. Created attachment 387921 [details]
Patch for landing
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 on attachment 387921 [details] Patch for landing Clearing flags on attachment: 387921 Committed r254688: <https://trac.webkit.org/changeset/254688> All reviewed patches have been landed. Closing bug. Reverted r254688 for reason: This caused internal test crashes. Committed r254778: <https://trac.webkit.org/changeset/254778> Created attachment 388216 [details]
Patch for landing
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 on attachment 388216 [details] Patch for landing Clearing flags on attachment: 388216 Committed r254817: <https://trac.webkit.org/changeset/254817> All reviewed patches have been landed. Closing bug. |