WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
125157
[MSE][Mac] Support painting MSE video-element to canvas
https://bugs.webkit.org/show_bug.cgi?id=125157
Summary
[MSE][Mac] Support painting MSE video-element to canvas
Jer Noble
Reported
2013-12-03 08:53:24 PST
[MSE][Mac] Support painting MSE video-element to canvas
Attachments
Patch
(124.64 KB, patch)
2016-02-29 11:29 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(124.72 KB, patch)
2016-03-01 08:45 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(124.73 KB, patch)
2016-03-01 09:17 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-yosemite
(1.38 MB, application/zip)
2016-03-01 10:02 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2
(1.05 MB, application/zip)
2016-03-01 10:06 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews112 for mac-yosemite
(1.35 MB, application/zip)
2016-03-01 10:20 PST
,
Build Bot
no flags
Details
Patch
(125.87 KB, patch)
2016-03-01 11:13 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(126.03 KB, patch)
2016-03-02 08:27 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch rebased against ToT
(96.18 KB, patch)
2016-12-08 10:43 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch rebased against ToT
(121.05 KB, patch)
2016-12-08 10:49 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(8.47 KB, patch)
2017-03-21 16:36 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(100.55 KB, patch)
2017-04-10 08:55 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-elcapitan
(1.46 MB, application/zip)
2017-04-10 10:31 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews123 for ios-simulator-wk2
(873.88 KB, application/zip)
2017-04-10 10:33 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
(1.64 MB, application/zip)
2017-04-10 10:36 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews113 for mac-elcapitan
(2.27 MB, application/zip)
2017-04-10 10:45 PDT
,
Build Bot
no flags
Details
Patch
(100.77 KB, patch)
2017-04-10 11:05 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-elcapitan
(903.11 KB, application/zip)
2017-04-10 11:58 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
(1.61 MB, application/zip)
2017-04-10 12:05 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews113 for mac-elcapitan
(1.71 MB, application/zip)
2017-04-10 12:30 PDT
,
Build Bot
no flags
Details
Patch
(100.87 KB, patch)
2017-04-10 13:21 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-elcapitan
(1004.69 KB, application/zip)
2017-04-10 14:39 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
(1.12 MB, application/zip)
2017-04-10 14:44 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews116 for mac-elcapitan
(1.69 MB, application/zip)
2017-04-10 14:48 PDT
,
Build Bot
no flags
Details
Patch for landing
(95.18 KB, patch)
2017-04-14 12:37 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(95.56 KB, patch)
2017-04-14 13:29 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-elcapitan
(870.92 KB, application/zip)
2017-04-14 14:47 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
(1.13 MB, application/zip)
2017-04-14 15:07 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews116 for mac-elcapitan
(1.54 MB, application/zip)
2017-04-14 15:20 PDT
,
Build Bot
no flags
Details
Patch for landing
(97.63 KB, patch)
2017-05-18 15:49 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(105.03 KB, patch)
2017-05-18 16:28 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(100.93 KB, patch)
2017-05-18 18:22 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan
(941.82 KB, application/zip)
2017-05-18 19:07 PDT
,
WebKit Commit Bot
no flags
Details
Patch for landing
(101.05 KB, patch)
2017-05-18 20:47 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan
(810.36 KB, application/zip)
2017-05-18 21:55 PDT
,
WebKit Commit Bot
no flags
Details
Patch for landing
(101.27 KB, patch)
2017-05-18 22:10 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(112.50 KB, patch)
2017-05-19 22:19 PDT
,
Jer Noble
jer.noble
: commit-queue+
Details
Formatted Diff
Diff
Show Obsolete
(28)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2015-10-10 19:06:45 PDT
<
rdar://problem/23062016
>
Alexey Proskuryakov
Comment 2
2015-11-19 09:02:10 PST
What is the proper way to track cross-origin/tainting in this case?
Jer Noble
Comment 3
2015-11-19 09:04:08 PST
For a first pass, we would have to assume that all painting from MSE taints the canvas. Second pass, taint the canvas if any non-single-origin XHR requests were made. Third pass, attach the single-origin state to the ArrayBuffer generated by XHRs, and ferry that data through MSE to the video element.
Jer Noble
Comment 4
2016-02-12 15:36:42 PST
***
Bug 154189
has been marked as a duplicate of this bug. ***
Daniel Rossi
Comment 5
2016-02-13 14:55:34 PST
This is a matter of leaving things to the last minute. An accident waiting to happen. It is related to the CORS flaw. Once CORS support is enabled , MediaSource WebGL textures will still be a problem. A very basic usage. See this for reference
https://bugs.webkit.org/show_bug.cgi?id=135379
My proxy hack work around does not work on Dash only mp4 files.
Daniel Rossi
Comment 6
2016-02-13 14:56:47 PST
CORS is needed because in the real world people use CDN's for video with sub domain cnames which break CORS.
Daniel Rossi
Comment 7
2016-02-13 20:02:51 PST
I have left a comment here. These two are related and nobody is assigned to the actual CORS flaw issue
https://bugs.webkit.org/show_bug.cgi?id=135379#c43
Daniel Rossi
Comment 8
2016-02-15 04:03:36 PST
I got myself confused. It seems when drawing to 2D canvas there is no CORS restriction it obviously does not use CORS. It's only when you attempt to get a data uri to get an image from the video does CORS become a problem which requires the proxy hack. However when trying to use WebGL there is and because Safari lacks CORS support , we have this issue. Here is a raw webgl example just with an mp4 file.
https://jsfiddle.net/7t77rz6L/11/
"SecurityError: DOM Exception 18: An attempt was made to break through the security policy of the user agent."
Daniel Rossi
Comment 9
2016-02-15 06:43:48 PST
Just confirming even with the proxy hack, Iphone will not play inline. It needs serious dodgy hacks of updating the loaded video time and separate audio. No good for webgl / VR needing to display the canvas. That same proxy hack to work around CORS does not work on MediaSource however.
Jer Noble
Comment 10
2016-02-29 11:29:46 PST
Created
attachment 272504
[details]
Patch
WebKit Commit Bot
Comment 11
2016-02-29 11:31:05 PST
Attachment 272504
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Daniel Rossi
Comment 12
2016-03-01 00:53:54 PST
That dependancy bug is fixed. Does that means CORS and webgl will play nicely soon ? Absolutely everyone in VR is depending on this minus the Iphone inline issue, not sure how to tackle that one, some nasty hacks out there. Apple can't market VR until this is done really but Iphone still unusable.
Jer Noble
Comment 13
2016-03-01 08:42:08 PST
(In reply to
comment #12
)
> That dependancy bug is fixed. Does that means CORS and webgl will play > nicely soon ?
This bug will only address the painting to canvas use case. There is still more work to enable efficient rendering to WebGL.
Jer Noble
Comment 14
2016-03-01 08:45:51 PST
Created
attachment 272565
[details]
Patch
WebKit Commit Bot
Comment 15
2016-03-01 08:47:46 PST
Attachment 272565
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Jer Noble
Comment 16
2016-03-01 09:17:29 PST
Created
attachment 272566
[details]
Patch
WebKit Commit Bot
Comment 17
2016-03-01 09:18:49 PST
Attachment 272566
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 18
2016-03-01 10:02:39 PST
Comment on
attachment 272566
[details]
Patch
Attachment 272566
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/906835
New failing tests: http/tests/media/media-source/mediasource-play.html http/tests/media/media-source/mediasource-config-change-mp4-v-framerate.html http/tests/media/media-source/mediasource-sourcebuffer-mode.html http/tests/media/media-source/mediasource-remove.html media/media-source/media-source-stalled-holds-sleep-assertion.html http/tests/media/media-source/mediasource-append-buffer.html http/tests/media/media-source/mediasource-config-change-mp4-v-bitrate.html media/media-source/media-source-paint-to-canvas.html
Build Bot
Comment 19
2016-03-01 10:02:42 PST
Created
attachment 272568
[details]
Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 20
2016-03-01 10:06:25 PST
Comment on
attachment 272566
[details]
Patch
Attachment 272566
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/906843
New failing tests: http/tests/media/media-source/mediasource-play.html http/tests/media/media-source/mediasource-config-change-mp4-v-framerate.html http/tests/media/media-source/mediasource-sourcebuffer-mode.html http/tests/media/media-source/mediasource-remove.html media/media-source/media-source-stalled-holds-sleep-assertion.html http/tests/media/media-source/mediasource-append-buffer.html http/tests/media/media-source/mediasource-config-change-mp4-v-bitrate.html media/media-source/media-source-paint-to-canvas.html
Build Bot
Comment 21
2016-03-01 10:06:29 PST
Created
attachment 272570
[details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Jer Noble
Comment 22
2016-03-01 10:11:50 PST
Crashes are an ASSERT in initVTDecompressionSessionDecodeFrameWithOutputHandler(...). Looking at the headers, I see __OSX_AVAILABLE_STARTING(__MAC_10_11,__IPHONE_9_0); Looks like I'll need to move things around to use VTDecompressionSessionDecodeFrame instead.
Build Bot
Comment 23
2016-03-01 10:19:59 PST
Comment on
attachment 272566
[details]
Patch
Attachment 272566
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/906852
New failing tests: http/tests/media/media-source/mediasource-play.html http/tests/media/media-source/mediasource-config-change-mp4-v-framerate.html http/tests/media/media-source/mediasource-sourcebuffer-mode.html http/tests/media/media-source/mediasource-remove.html media/media-source/media-source-stalled-holds-sleep-assertion.html http/tests/media/media-source/mediasource-append-buffer.html http/tests/media/media-source/mediasource-config-change-mp4-v-bitrate.html media/media-source/media-source-paint-to-canvas.html
Build Bot
Comment 24
2016-03-01 10:20:03 PST
Created
attachment 272572
[details]
Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Jer Noble
Comment 25
2016-03-01 11:13:35 PST
Created
attachment 272576
[details]
Patch
Jer Noble
Comment 26
2016-03-02 08:27:25 PST
Created
attachment 272657
[details]
Patch
Eric Carlson
Comment 27
2016-03-02 09:44:48 PST
Comment on
attachment 272657
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=272657&action=review
r=me wth some nits and restructuring suggestions.
> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:580 > + m_player->firstVideoFrameAvailable();
Do you want to call firstVideoFrameAvailable more than once in the player's lifecycle?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:611 > +class WebCoreDecompressionSession : public ThreadSafeRefCounted<WebCoreDecompressionSession> { > +public: > + static Ref<WebCoreDecompressionSession> create(AVSampleBufferDisplayLayer* layer, AVSampleBufferRenderSynchronizer *synchronizer) { return adoptRef(*new WebCoreDecompressionSession(layer, synchronizer)); }
Is there any reason to hide this here in SourceBufferPrivateAVFObjC.mm? It doesn't look like this class is MSE-specific in any way, and it could be extremely useful for other media engines (eg. MediaStream).
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:650 > + std::function<void()> m_notificationCallback;
Nit: This name is really generic for a callback with a specific purpose. Maybe m_requestMediaDataCallback?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:700 > + timingInfoArray.grow(itemCount);
Why .grow() instead of .reserveCapacity(), or Vector<CMSampleTimingInfo> vector(itemCount)?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:720 > + CMBufferQueueCreate(kCFAllocatorDefault, kMaximumCapacity, &callbacks, &outQueue);
Nit: kMaximumQueueCapacity would be clearer.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:723 > + CMBufferQueueInstallTriggerWithIntegerThreshold(m_producerQueue.get(), maybeBecomeReadyForMoreMediaDataCallback, this, kCMBufferQueueTrigger_WhenBufferCountBecomesLessThan, 15, &m_didBecomeReadyTrigger);
This magic number should be a named constant to make it easier to understand what it means.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:760 > + if (m_decompressionSession && !VTDecompressionSessionCanAcceptFormatDescription(m_decompressionSession.get(), videoFormatDescription)) { > + VTDecompressionSessionWaitForAsynchronousFrames(m_decompressionSession.get()); > + m_decompressionSession = nullptr; > + }
Nit: you should definitely add error logging here to make it easier to diagnose failures.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770 > + VTDecompressionOutputCallbackRecord callback { > + &decompressionOutputCallback, > + this, > + };
What guarantees that "this" stays alive? Should it be a weakPtr?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:794 > + if (noErr != CMVideoFormatDescriptionCreateForImageBuffer(kCFAllocatorDefault, rawImageBuffer, &rawImageBufferDescription)) > + return;
You should log this error.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:805 > + if (noErr != CMSampleBufferCreateForImageBuffer(kCFAllocatorDefault, rawImageBuffer, true, nullptr, nullptr, imageBufferDescription.get(), &imageBufferTiming, &rawImageSampleBuffer)) > + return;
Ditto.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:809 > + callOnMainThread([strongThis, status, imageSampleBuffer, infoFlags, displaying] { > + UNUSED_PARAM(infoFlags);
Why pass infoFlags if you don't use them?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:819 > + --m_framesBeingDecoded;
ASSERT(m_framesBeingDecoded >= 0)
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:858 > + RetainPtr<CMSampleBufferRef> sample = adoptCF((CMSampleBufferRef)CMBufferQueueDequeueAndRetain(m_producerQueue.get())); > + [m_displayLayer enqueueSampleBuffer:sample.get()]; > + CMBufferQueueEnqueue(m_consumerQueue.get(), sample.get());
Is this correct if [m_displayLayer isReadyForMoreMediaData] above returned false?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:935 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf); > + return CMSampleBufferGetDecodeTimeStamp(sample);
Nit: you don't need the local variable.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:942 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf); > + return CMSampleBufferGetPresentationTimeStamp(sample);
Ditto.
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:949 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf); > + return CMSampleBufferGetDuration(sample);
Ditto.
Jer Noble
Comment 28
2016-03-02 10:49:30 PST
Comment on
attachment 272657
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=272657&action=review
>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:580 >> + m_player->firstVideoFrameAvailable(); > > Do you want to call firstVideoFrameAvailable more than once in the player's lifecycle?
Yes. This lets the media element know to switch from displaying the poster frame to displaying the video. And this is not limited to happening only once per player. If this player is seeked out of it's buffered range, it could not have flushed all its images and have nothing to display, in which case the media element will have switched back to displaying the poster. Perhaps we need to rename the client method though.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:611 >> + static Ref<WebCoreDecompressionSession> create(AVSampleBufferDisplayLayer* layer, AVSampleBufferRenderSynchronizer *synchronizer) { return adoptRef(*new WebCoreDecompressionSession(layer, synchronizer)); } > > Is there any reason to hide this here in SourceBufferPrivateAVFObjC.mm? It doesn't look like this class is MSE-specific in any way, and it could be extremely useful for other media engines (eg. MediaStream).
This could get pulled into its own file, sure.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:650 >> + std::function<void()> m_notificationCallback; > > Nit: This name is really generic for a callback with a specific purpose. Maybe m_requestMediaDataCallback?
Sure.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:700 >> + timingInfoArray.grow(itemCount); > > Why .grow() instead of .reserveCapacity(), or Vector<CMSampleTimingInfo> vector(itemCount)?
The latter isn't possible; you need to pass in a `const CMSampleTimingInfo&` in addition to a itemCount, and setting specific values is unnecessary since the next method will overwrite the contents anyway. And while the former resizes the Vector's buffer, it doesn't resize the vector itself. Of course, now that I read this, I can't remember why I needed the sample timing at all. I'm pretty sure I can drop this whole section.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:720 >> + CMBufferQueueCreate(kCFAllocatorDefault, kMaximumCapacity, &callbacks, &outQueue); > > Nit: kMaximumQueueCapacity would be clearer.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:723 >> + CMBufferQueueInstallTriggerWithIntegerThreshold(m_producerQueue.get(), maybeBecomeReadyForMoreMediaDataCallback, this, kCMBufferQueueTrigger_WhenBufferCountBecomesLessThan, 15, &m_didBecomeReadyTrigger); > > This magic number should be a named constant to make it easier to understand what it means.
You're right, this is supposed to be kLowWaterMark.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:760 >> + } > > Nit: you should definitely add error logging here to make it easier to diagnose failures.
This is definitely not an error condition. If the client starts pushing in different dimension samples, we may need to restart the decompression session.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770 >> + }; > > What guarantees that "this" stays alive? Should it be a weakPtr?
Unfortunately, it can't be. It has to be a bare pointer. However, we're not using asynchronous decoding flags when we decode frames, so the callback will occur before VTDecompressionSessionDecodeFrame() returns. So there's no danger here.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:794 >> + return; > > You should log this error.
ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:805 >> + return; > > Ditto.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:809 >> + UNUSED_PARAM(infoFlags); > > Why pass infoFlags if you don't use them?
Good point. :)
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:819 >> + --m_framesBeingDecoded; > > ASSERT(m_framesBeingDecoded >= 0)
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:858 >> + CMBufferQueueEnqueue(m_consumerQueue.get(), sample.get()); > > Is this correct if [m_displayLayer isReadyForMoreMediaData] above returned false?
This won't be reached if -isReadyForMoreMediaData returns false.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:935 >> + return CMSampleBufferGetDecodeTimeStamp(sample); > > Nit: you don't need the local variable.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:942 >> + return CMSampleBufferGetPresentationTimeStamp(sample); > > Ditto.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:949 >> + return CMSampleBufferGetDuration(sample); > > Ditto.
Ok.
Jer Noble
Comment 29
2016-03-02 10:49:32 PST
Comment on
attachment 272657
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=272657&action=review
>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:580 >> + m_player->firstVideoFrameAvailable(); > > Do you want to call firstVideoFrameAvailable more than once in the player's lifecycle?
Yes. This lets the media element know to switch from displaying the poster frame to displaying the video. And this is not limited to happening only once per player. If this player is seeked out of it's buffered range, it could not have flushed all its images and have nothing to display, in which case the media element will have switched back to displaying the poster. Perhaps we need to rename the client method though.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:611 >> + static Ref<WebCoreDecompressionSession> create(AVSampleBufferDisplayLayer* layer, AVSampleBufferRenderSynchronizer *synchronizer) { return adoptRef(*new WebCoreDecompressionSession(layer, synchronizer)); } > > Is there any reason to hide this here in SourceBufferPrivateAVFObjC.mm? It doesn't look like this class is MSE-specific in any way, and it could be extremely useful for other media engines (eg. MediaStream).
This could get pulled into its own file, sure.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:650 >> + std::function<void()> m_notificationCallback; > > Nit: This name is really generic for a callback with a specific purpose. Maybe m_requestMediaDataCallback?
Sure.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:700 >> + timingInfoArray.grow(itemCount); > > Why .grow() instead of .reserveCapacity(), or Vector<CMSampleTimingInfo> vector(itemCount)?
The latter isn't possible; you need to pass in a `const CMSampleTimingInfo&` in addition to a itemCount, and setting specific values is unnecessary since the next method will overwrite the contents anyway. And while the former resizes the Vector's buffer, it doesn't resize the vector itself. Of course, now that I read this, I can't remember why I needed the sample timing at all. I'm pretty sure I can drop this whole section.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:720 >> + CMBufferQueueCreate(kCFAllocatorDefault, kMaximumCapacity, &callbacks, &outQueue); > > Nit: kMaximumQueueCapacity would be clearer.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:723 >> + CMBufferQueueInstallTriggerWithIntegerThreshold(m_producerQueue.get(), maybeBecomeReadyForMoreMediaDataCallback, this, kCMBufferQueueTrigger_WhenBufferCountBecomesLessThan, 15, &m_didBecomeReadyTrigger); > > This magic number should be a named constant to make it easier to understand what it means.
You're right, this is supposed to be kLowWaterMark.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:760 >> + } > > Nit: you should definitely add error logging here to make it easier to diagnose failures.
This is definitely not an error condition. If the client starts pushing in different dimension samples, we may need to restart the decompression session.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770 >> + }; > > What guarantees that "this" stays alive? Should it be a weakPtr?
Unfortunately, it can't be. It has to be a bare pointer. However, we're not using asynchronous decoding flags when we decode frames, so the callback will occur before VTDecompressionSessionDecodeFrame() returns. So there's no danger here.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:794 >> + return; > > You should log this error.
ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:805 >> + return; > > Ditto.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:809 >> + UNUSED_PARAM(infoFlags); > > Why pass infoFlags if you don't use them?
Good point. :)
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:819 >> + --m_framesBeingDecoded; > > ASSERT(m_framesBeingDecoded >= 0)
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:858 >> + CMBufferQueueEnqueue(m_consumerQueue.get(), sample.get()); > > Is this correct if [m_displayLayer isReadyForMoreMediaData] above returned false?
This won't be reached if -isReadyForMoreMediaData returns false.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:935 >> + return CMSampleBufferGetDecodeTimeStamp(sample); > > Nit: you don't need the local variable.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:942 >> + return CMSampleBufferGetPresentationTimeStamp(sample); > > Ditto.
Ok.
>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:949 >> + return CMSampleBufferGetDuration(sample); > > Ditto.
Ok.
Daniel Rossi
Comment 30
2016-04-06 02:27:09 PDT
Any ideas of the status of this ? I am hoping this means there is CORS support in webkit after this ?
Daniel Rossi
Comment 31
2016-05-13 04:02:38 PDT
Hi guys this has gone quiet. VR people are waiting for this fix and it's pretty critical. Already missed the boat it seems ! I'm going to see if running in WebView / Cordova works around this lack of implementation like it can override inline restrictions.
Daniel Rossi
Comment 32
2016-06-03 08:15:06 PDT
Any ideas of the status of this ? I know it's pretty much crept up and steamrolled, many people trying to do WebVR including Youtube and Facebook of all people are also waiting. They should pay to make this happen. This problem has been left exposed for many years now and was just an accident waiting to happen. For some reason a few of us have to do the run around for many others. Obviously the reverse proxy work around is not ok for production so not even Facebook or Youtube are doing that and therefore Safari is broken. If there is a nightly fix to test please let me know.
Alexandre Buisine
Comment 33
2016-06-03 08:22:24 PDT
I am interested as well in any experimental version and would be happy to run a bunch of test on it ! :)
Alexey Proskuryakov
Comment 34
2016-06-03 09:56:06 PDT
Please see <
https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
>.
Dean Jackson
Comment 35
2016-06-03 10:04:49 PDT
Sorry, no progress to report on this bug at the moment.
Daniel Rossi
Comment 36
2016-06-14 18:58:58 PDT
Hi guys I'm sorry to use this as a forum. But by stealth it seems Apple have forked the code and fixed it correct ? This is not so useful for current browsers so wondering if they will merge it back ?
https://twitter.com/zenoc/status/742770789880111104
Dean Jackson
Comment 37
2016-06-15 10:29:48 PDT
(In reply to
comment #36
)
> Hi guys I'm sorry to use this as a forum. But by stealth it seems Apple have > forked the code and fixed it correct ? > > This is not so useful for current browsers so wondering if they will merge > it back ? > >
https://twitter.com/zenoc/status/742770789880111104
This only applies to videos on macOS running Sierra.
Daniel Rossi
Comment 38
2016-09-22 03:24:51 PDT
Yes I am aware they are only interested in fixing it in an OS nobody even has yet. Which renders all other OS useless. However I've discovered a quiet change in Safari in Yosemite which has broken feature detection. Something truly bizarre has changed in Safari on Yosemite that was not there before. It has broken feature detection with CORS in alot of things I have done. Just a heads up people just working how broken it really is. ``` var testVideo = document.createElement("video"); testVideo.crossOrigin = "anonymous"; console.log(testVideo.hasAttribute("crossOrigin")); ``` In IE11 it returns false as expected. In Safari it's true but the CORS bug is still obviously there. The feature detection is there to obviously require the proxy hack. Madness. So I can't help to think some half baked change was pushed ?
Jer Noble
Comment 39
2016-12-08 10:43:39 PST
Created
attachment 296534
[details]
Patch rebased against ToT
Jer Noble
Comment 40
2016-12-08 10:49:30 PST
Created
attachment 296535
[details]
Patch rebased against ToT
Jon Lee
Comment 41
2017-03-03 14:56:16 PST
rdar://problem/22925644
Jer Noble
Comment 42
2017-03-21 16:36:01 PDT
Created
attachment 305047
[details]
Patch for landing
Jer Noble
Comment 43
2017-04-10 08:55:13 PDT
Created
attachment 306690
[details]
Patch
Build Bot
Comment 44
2017-04-10 10:31:17 PDT
Comment on
attachment 306690
[details]
Patch
Attachment 306690
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3512016
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-resize.html media/video-source-error-no-candidate.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/src_reflects_attribute_not_source_elements.html media/remove-from-document-before-load.html media/W3C/video/error/error_onerror_called_on_bogus_source.html http/tests/media/text-served-as-text.html media/media-source/media-source-fastseek.html media/video-src-change.html media/media-source/media-source-remove.html media/invalid-media-url-crash.html http/tests/media/pdf-served-as-pdf.html media/media-source/media-source-seek-detach-crash.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html imported/w3c/web-platform-tests/media-source/mediasource-preload.html
Build Bot
Comment 45
2017-04-10 10:31:19 PDT
Created
attachment 306704
[details]
Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 46
2017-04-10 10:33:17 PDT
Comment on
attachment 306690
[details]
Patch
Attachment 306690
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/3511968
New failing tests: scrollingcoordinator/ios/sync-layer-positions-after-scroll.html
Build Bot
Comment 47
2017-04-10 10:33:18 PDT
Created
attachment 306706
[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 48
2017-04-10 10:36:14 PDT
Comment on
attachment 306690
[details]
Patch
Attachment 306690
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3512031
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-resize.html media/video-source-error-no-candidate.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/src_reflects_attribute_not_source_elements.html media/remove-from-document-before-load.html media/W3C/video/error/error_onerror_called_on_bogus_source.html http/tests/media/text-served-as-text.html media/media-source/media-source-fastseek.html media/video-src-change.html media/media-source/media-source-remove.html media/invalid-media-url-crash.html http/tests/media/pdf-served-as-pdf.html media/media-source/media-source-seek-detach-crash.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html imported/w3c/web-platform-tests/media-source/mediasource-preload.html
Build Bot
Comment 49
2017-04-10 10:36:15 PDT
Created
attachment 306707
[details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 50
2017-04-10 10:45:50 PDT
Comment on
attachment 306690
[details]
Patch
Attachment 306690
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3512051
New failing tests: http/tests/media/text-served-as-text.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html media/video-source-error-no-candidate.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/src_reflects_attribute_not_source_elements.html media/remove-from-document-before-load.html media/W3C/video/error/error_onerror_called_on_bogus_source.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/video-src-change.html media/media-source/media-source-remove.html media/invalid-media-url-crash.html http/tests/media/pdf-served-as-pdf.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html imported/w3c/web-platform-tests/media-source/mediasource-preload.html
Build Bot
Comment 51
2017-04-10 10:45:51 PDT
Created
attachment 306709
[details]
Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 52
2017-04-10 11:05:45 PDT
Created
attachment 306715
[details]
Patch
Build Bot
Comment 53
2017-04-10 11:58:01 PDT
Comment on
attachment 306715
[details]
Patch
Attachment 306715
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3512521
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 54
2017-04-10 11:58:02 PDT
Created
attachment 306725
[details]
Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 55
2017-04-10 12:05:07 PDT
Comment on
attachment 306715
[details]
Patch
Attachment 306715
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3512536
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 56
2017-04-10 12:05:09 PDT
Created
attachment 306726
[details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 57
2017-04-10 12:30:38 PDT
Comment on
attachment 306715
[details]
Patch
Attachment 306715
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3512636
New failing tests: media/media-source/media-source-resize.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html
Build Bot
Comment 58
2017-04-10 12:30:39 PDT
Created
attachment 306730
[details]
Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 59
2017-04-10 13:21:20 PDT
Created
attachment 306734
[details]
Patch
Build Bot
Comment 60
2017-04-10 14:39:28 PDT
Comment on
attachment 306734
[details]
Patch
Attachment 306734
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3513191
New failing tests: media/media-source/media-source-resize.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html
Build Bot
Comment 61
2017-04-10 14:39:30 PDT
Created
attachment 306744
[details]
Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 62
2017-04-10 14:44:41 PDT
Comment on
attachment 306734
[details]
Patch
Attachment 306734
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3513199
New failing tests: media/media-source/media-source-resize.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html
Build Bot
Comment 63
2017-04-10 14:44:42 PDT
Created
attachment 306746
[details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 64
2017-04-10 14:48:24 PDT
Comment on
attachment 306734
[details]
Patch
Attachment 306734
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3513197
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-fastseek.html media/media-source/media-source-remove.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 65
2017-04-10 14:48:25 PDT
Created
attachment 306747
[details]
Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Eric Carlson
Comment 66
2017-04-11 08:44:53 PDT
Comment on
attachment 306734
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=306734&action=review
> Source/WebCore/ChangeLog:108 > + * platform/cocoa/VideoToolboxSoftLink.cpp: Added. > + * platform/cocoa/VideoToolboxSoftLink.h: Added. > + > +2016-11-29 Jer Noble <
jer.noble@apple.com
> > + > + [MSE][Mac] Support painting MSE video-element to canvas > +
https://bugs.webkit.org/show_bug.cgi?id=125157
> + <
rdar://problem/23062016
>
Double ChangeLog.
> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:538 > + RetainPtr<CVPixelBufferRef> pixelBuffer = m_decompressionSession->imageForTime(currentMediaTime(), flags);
auto?
> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:871 > + if (m_decompressionSession) { > + m_decompressionSession->flush(); > + m_decompressionSession->notifyWhenHasAvailableVideoFrame([weakThis = createWeakPtr()] { > + if (weakThis && weakThis->m_mediaSource) > + weakThis->m_mediaSource->player()->setHasAvailableVideoFrame(true); > + }); > + }
Nit: trackDidChangeEnabled has the same code. Split into a helper function?
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:51 > + auto weakThis = createWeakPtr();
Is this still necessary?
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:64 > + WebCoreDecompressionSession* session = static_cast<WebCoreDecompressionSession*>(refcon); > + session->maybeBecomeReadyForMoreMediaData();
Nit: the local variable isn't necessary.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:109 > + CMBufferQueueInstallTriggerWithIntegerThreshold(m_producerQueue.get(), maybeBecomeReadyForMoreMediaDataCallback, this, kCMBufferQueueTrigger_WhenBufferCountBecomesLessThan, 15, &m_didBecomeReadyTrigger);
Why "15", is it kHighWaterMark/2. A named constant would be nice.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:149 > + CMVideoFormatDescriptionRef videoFormatDescription = CMSampleBufferGetFormatDescription(sample);
auto?
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:165 > + VTDecompressionSessionDecodeFrame(m_decompressionSession.get(), sample, 0, reinterpret_cast<void*>(displaying), &flags);
An enum would be easier to follow than a bool.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:180 > + RetainPtr<CMVideoFormatDescriptionRef> imageBufferDescription = adoptCF(rawImageBufferDescription);
auto?
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:204 > + RetainPtr<CMSampleBufferRef> currentSample = adoptCF((CMSampleBufferRef)CMBufferQueueDequeueAndRetain(m_producerQueue.get()));
Ditto.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:215 > + --m_framesBeingDecoded;
ASSERT(m_framesBeingDecoded >= 0)
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:240 > +void WebCoreDecompressionSession::requestMediaDataWhenReady(std::function<void()> notificationCallback)
It doesn't look like notificationCallback is ever null, so std::function<void()>&& notificationCallback
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:242 > + m_notificationCallback = notificationCallback;
and m_notificationCallback = WTFMove(notificationCallback);
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:258 > +void WebCoreDecompressionSession::notifyWhenHasAvailableVideoFrame(std::function<void()> callback)
Ditto.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:261 > + dispatch_async(dispatch_get_main_queue(), [callback] {
dispatch_async(dispatch_get_main_queue(), [callback = WTFMove(callback)] {
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:266 > + m_hasAvailableFrameCallback = callback;
m_hasAvailableFrameCallback = WTFMove(callback)
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:278 > + MediaTime startTime = toMediaTime(CMBufferQueueGetFirstDecodeTimeStamp(m_producerQueue.get())); > + MediaTime endTime = toMediaTime(CMBufferQueueGetEndPresentationTimeStamp(m_producerQueue.get()));
auto?
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:287 > + while (CMSampleBufferRef firstSample = (CMSampleBufferRef)CMBufferQueueGetHead(m_producerQueue.get())) { > + MediaTime presentationTimestamp = toMediaTime(CMSampleBufferGetPresentationTimeStamp(firstSample)); > + MediaTime duration = toMediaTime(CMSampleBufferGetDuration(firstSample));
Ditto.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:297 > + RetainPtr<CMSampleBufferRef> currentSample = adoptCF((CMSampleBufferRef)CMBufferQueueDequeueAndRetain(m_producerQueue.get())); > + RetainPtr<CVPixelBufferRef> imageBuffer = (CVPixelBufferRef)CMSampleBufferGetImageBuffer(currentSample.get());
Ditto.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:317 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf);
Nit: local isn't necessary.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:324 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf);
Ditto.
> Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:331 > + CMSampleBufferRef sample = (CMSampleBufferRef)(buf);
Ditto.
Jer Noble
Comment 67
2017-04-14 12:37:16 PDT
Created
attachment 307134
[details]
Patch for landing
Jer Noble
Comment 68
2017-04-14 13:29:39 PDT
Created
attachment 307140
[details]
Patch for landing
Build Bot
Comment 69
2017-04-14 14:47:36 PDT
Comment on
attachment 307140
[details]
Patch for landing
Attachment 307140
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3535641
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 70
2017-04-14 14:47:38 PDT
Created
attachment 307148
[details]
Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 71
2017-04-14 15:07:20 PDT
Comment on
attachment 307140
[details]
Patch for landing
Attachment 307140
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3535697
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 72
2017-04-14 15:07:22 PDT
Created
attachment 307151
[details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 73
2017-04-14 15:20:51 PDT
Comment on
attachment 307140
[details]
Patch for landing
Attachment 307140
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3535737
New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-seek-detach-crash.html media/media-source/media-source-resize.html
Build Bot
Comment 74
2017-04-14 15:20:53 PDT
Created
attachment 307153
[details]
Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 75
2017-05-18 15:49:47 PDT
Created
attachment 310564
[details]
Patch for landing
WebKit Commit Bot
Comment 76
2017-05-18 16:18:23 PDT
Comment on
attachment 310564
[details]
Patch for landing Rejecting
attachment 310564
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: d/Objects-normal/x86_64/VideoToolboxSoftLink.o clang: error: no such file or directory: '/Volumes/Data/EWS/WebKit/Source/WebCore/platform/cocoa/VideoToolboxSoftLink.cpp' clang: error: no input files ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/VideoToolboxSoftLink.o platform/cocoa/VideoToolboxSoftLink.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output:
http://webkit-queues.webkit.org/results/3772093
Jer Noble
Comment 77
2017-05-18 16:28:08 PDT
Created
attachment 310573
[details]
Patch for landing
WebKit Commit Bot
Comment 78
2017-05-18 16:45:32 PDT
Comment on
attachment 310573
[details]
Patch for landing Rejecting
attachment 310573
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: S/WebKit/Source/WebCore/html/track/TextTrackList.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/TextTrackList.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/WebCoreDecompressionSession.o platform/graphics/cocoa/WebCoreDecompressionSession.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output:
http://webkit-queues.webkit.org/results/3772338
Jer Noble
Comment 79
2017-05-18 18:22:33 PDT
Created
attachment 310586
[details]
Patch for landing
WebKit Commit Bot
Comment 80
2017-05-18 19:07:45 PDT
Comment on
attachment 310586
[details]
Patch for landing Rejecting
attachment 310586
[details]
from commit-queue. New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html media/media-source/media-source-remove.html media/media-source/media-source-fastseek.html media/media-source/media-source-sequence-timestamps.html imported/w3c/web-platform-tests/media-source/mediasource-avtracks.html Full output:
http://webkit-queues.webkit.org/results/3772975
WebKit Commit Bot
Comment 81
2017-05-18 19:07:47 PDT
Created
attachment 310591
[details]
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-03 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 82
2017-05-18 20:47:39 PDT
Created
attachment 310603
[details]
Patch for landing
WebKit Commit Bot
Comment 83
2017-05-18 21:55:14 PDT
Comment on
attachment 310603
[details]
Patch for landing Rejecting
attachment 310603
[details]
from commit-queue. New failing tests: imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html Full output:
http://webkit-queues.webkit.org/results/3773766
WebKit Commit Bot
Comment 84
2017-05-18 21:55:16 PDT
Created
attachment 310611
[details]
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-03 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 85
2017-05-18 22:10:34 PDT
Created
attachment 310613
[details]
Patch for landing
WebKit Commit Bot
Comment 86
2017-05-18 22:50:39 PDT
Comment on
attachment 310613
[details]
Patch for landing Clearing flags on attachment: 310613 Committed
r217098
: <
http://trac.webkit.org/changeset/217098
>
WebKit Commit Bot
Comment 87
2017-05-18 22:50:42 PDT
All reviewed patches have been landed. Closing bug.
Jer Noble
Comment 88
2017-05-19 02:42:24 PDT
Landed test in: Committed
r217111
: <
http://trac.webkit.org/changeset/217111
>
Jer Noble
Comment 89
2017-05-19 02:51:45 PDT
Landed build fix in: Committed
r217113
: <
http://trac.webkit.org/changeset/217113
>
Jer Noble
Comment 90
2017-05-19 02:55:17 PDT
Landed one more test fix in: Committed
r217114
: <
http://trac.webkit.org/changeset/217114
>
Ryan Haddad
Comment 91
2017-05-19 10:54:59 PDT
These changes broke Windows build:
https://build.webkit.org/builders/Apple%20Win%20Release%20(Build)/builds/1639
There are also media test issues: Crash: http/tests/media/video-play-stall.html Failures: media/media-source/media-source-paint-to-canvas.html media/media-source/media-source-resize.html Timeout: media/media-source/media-source-seek-detach-crash.html
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r217128%20(1618)/results.html
Ryan Haddad
Comment 92
2017-05-19 11:02:08 PDT
This is making all of the testers red and causing serious issues for EWS, so I'm going to roll out these changes.
WebKit Commit Bot
Comment 93
2017-05-19 11:03:18 PDT
Re-opened since this is blocked by
bug 172367
Jer Noble
Comment 94
2017-05-19 22:19:07 PDT
Created
attachment 310761
[details]
Patch for landing
WebKit Commit Bot
Comment 95
2017-05-20 09:55:05 PDT
Comment on
attachment 310761
[details]
Patch for landing Clearing flags on attachment: 310761 Committed
r217185
: <
http://trac.webkit.org/changeset/217185
>
WebKit Commit Bot
Comment 96
2017-05-20 09:55:08 PDT
All reviewed patches have been landed. Closing bug.
tomerlahav
Comment 97
2020-01-26 15:30:54 PST
I might be doing something wrong here, but this doesn't seem to work on Safari 13. Made a simple test page here:
https://s3.amazonaws.com/storage2.interlude.fm/dev_temp/tomer/safari_mse_canvas_bug/index.html
The above test page works correctly on other browsers (including IE), but MSE video is not drawn onto the canvas on Safari 13 (desktop + iPadOS). Would appreciate any pointers - thanks!!
Jon Lee
Comment 98
2020-01-26 15:45:09 PST
(In reply to tomerlahav from
comment #97
)
> I might be doing something wrong here, but this doesn't seem to work on > Safari 13. > Made a simple test page here: >
https://s3.amazonaws.com/storage2.interlude.fm/dev_temp/tomer/
> safari_mse_canvas_bug/index.html > > The above test page works correctly on other browsers (including IE), but > MSE video is not drawn onto the canvas on Safari 13 (desktop + iPadOS). > Would appreciate any pointers - thanks!!
Would you mind filing a new bug? It is hard to track the bug in an issue that was resolved.
tomerlahav
Comment 99
2020-01-26 16:32:34 PST
Sure thing, here it is:
https://bugs.webkit.org/show_bug.cgi?id=206812
Thanks!
Jer Noble
Comment 100
2022-02-08 21:03:00 PST
Update to this bug: it was rolled out because it caused a large memory regression in our performance tests. So we're back to the status-quo-ante, where painting of MSE content when the element is in the DOM is broken. The current best workaround is to remove the element from the DOM if you want to paint it to a canvas. Apologies for the late response here.
Daniel Rossi
Comment 101
2022-02-08 23:19:25 PST
I don't have a use case where MSE is in use as HLS falls back to native HLS. My target for Safari is native HLS. I am finding sometimes HLS refuses to paint at all until another page reload. On preload frame is black then continues like that. On Ipad IOS 15 this time. Would removing video from the dom help here too ?
Daniel Rossi
Comment 102
2022-02-11 02:36:18 PST
I played around with this and doing this with all browsers Chrome is pausing upon removal of the element for certain situations. MSE and mediastream for one. sad.
Daniel Rossi
Comment 103
2022-02-11 02:46:40 PST
In Safari in IOS mediastream sources also pauses when element is removed. Sad.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug