Title: [218463] trunk/Source/WebCore
- Revision
- 218463
- Author
- [email protected]
- Date
- 2017-06-18 08:55:55 -0700 (Sun, 18 Jun 2017)
Log Message
[MSE] Seeking or entering fullscreen can cause extreme CPU usage
https://bugs.webkit.org/show_bug.cgi?id=173505
Reviewed by Tim Horton.
When support for painting MSE to WebGL was added in r217185, the implementation of
SourceBufferPrivateAVFObjC::isReadyForMoreSamples() was modified to support asking
the decompression session if it was ready. That change, however, caused an extreme
performance regression in the normal playback path, where WebKit will effectively
append samples endlessly to the AVSampleBufferDisplayLayer, which admirably enqueued
each of them for decoding. Eventually, the cost of iterating over the CMBufferQueue
overwhelmed the cost of decoding, and caused the extreme lag seen when seeking.
Make sure to property query the AVSampleBufferDisplayLayer for isReadyForMoreMediaData
before enqueuing.
A previous version of this patch exposed some errors which caused failing tests:
In sourceBufferPrivateDidReceiveSample(), we were using local versions of
presentationTimestamp and decodeTimestamp as keys to the decodeQueue; those local versions
were floating point values (because MediaTime + float = float), but the sample itself uses
non-floating point MediaTimes. This causes samples to be left in the queue when they should
be removed.
In didBecomeReadyForMoreSamples(), we were getting spurious assertions when a
AVSampleBufferDisplayLayer or a AVSampleBufferAudioRenderer would fire a callback from
-requestMediaDataWhenReadyOnQueue:usingBlock: even after it had been told to
-stopRequestingMediaData. Apparently it's expected behavior and so an ASSERT_NOT_REACHED is
inappropriate here.
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::isReadyForMoreSamples):
(WebCore::SourceBufferPrivateAVFObjC::didBecomeReadyForMoreSamples):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218462 => 218463)
--- trunk/Source/WebCore/ChangeLog 2017-06-18 03:57:26 UTC (rev 218462)
+++ trunk/Source/WebCore/ChangeLog 2017-06-18 15:55:55 UTC (rev 218463)
@@ -1,3 +1,42 @@
+2017-06-18 Jer Noble <[email protected]>
+
+ [MSE] Seeking or entering fullscreen can cause extreme CPU usage
+ https://bugs.webkit.org/show_bug.cgi?id=173505
+
+ Reviewed by Tim Horton.
+
+ When support for painting MSE to WebGL was added in r217185, the implementation of
+ SourceBufferPrivateAVFObjC::isReadyForMoreSamples() was modified to support asking
+ the decompression session if it was ready. That change, however, caused an extreme
+ performance regression in the normal playback path, where WebKit will effectively
+ append samples endlessly to the AVSampleBufferDisplayLayer, which admirably enqueued
+ each of them for decoding. Eventually, the cost of iterating over the CMBufferQueue
+ overwhelmed the cost of decoding, and caused the extreme lag seen when seeking.
+
+ Make sure to property query the AVSampleBufferDisplayLayer for isReadyForMoreMediaData
+ before enqueuing.
+
+ A previous version of this patch exposed some errors which caused failing tests:
+
+ In sourceBufferPrivateDidReceiveSample(), we were using local versions of
+ presentationTimestamp and decodeTimestamp as keys to the decodeQueue; those local versions
+ were floating point values (because MediaTime + float = float), but the sample itself uses
+ non-floating point MediaTimes. This causes samples to be left in the queue when they should
+ be removed.
+
+ In didBecomeReadyForMoreSamples(), we were getting spurious assertions when a
+ AVSampleBufferDisplayLayer or a AVSampleBufferAudioRenderer would fire a callback from
+ -requestMediaDataWhenReadyOnQueue:usingBlock: even after it had been told to
+ -stopRequestingMediaData. Apparently it's expected behavior and so an ASSERT_NOT_REACHED is
+ inappropriate here.
+
+ * Modules/mediasource/SourceBuffer.cpp:
+ (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+ * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+ (WebCore::SourceBufferPrivateAVFObjC::isReadyForMoreSamples):
+ (WebCore::SourceBufferPrivateAVFObjC::didBecomeReadyForMoreSamples):
+
+
2017-06-17 Zalan Bujtas <[email protected]>
Addressing post-review comment after r218456.
Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (218462 => 218463)
--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2017-06-18 03:57:26 UTC (rev 218462)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2017-06-18 15:55:55 UTC (rev 218463)
@@ -1595,7 +1595,7 @@
trackBuffer.samples.addSample(sample);
if (trackBuffer.lastEnqueuedDecodeEndTime.isInvalid() || decodeTimestamp >= trackBuffer.lastEnqueuedDecodeEndTime) {
- DecodeOrderSampleMap::KeyType decodeKey(decodeTimestamp, presentationTimestamp);
+ DecodeOrderSampleMap::KeyType decodeKey(sample.decodeTime(), sample.presentationTime());
trackBuffer.decodeQueue.insert(DecodeOrderSampleMap::MapType::value_type(decodeKey, &sample));
}
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm (218462 => 218463)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2017-06-18 03:57:26 UTC (rev 218462)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm 2017-06-18 15:55:55 UTC (rev 218463)
@@ -1002,9 +1002,13 @@
bool SourceBufferPrivateAVFObjC::isReadyForMoreSamples(const AtomicString& trackIDString)
{
int trackID = trackIDString.toInt();
- if (trackID == m_enabledVideoTrackID)
- return !m_decompressionSession || m_decompressionSession->isReadyForMoreMediaData();
+ if (trackID == m_enabledVideoTrackID) {
+ if (m_decompressionSession)
+ return m_decompressionSession->isReadyForMoreMediaData();
+ return [m_displayLayer isReadyForMoreMediaData];
+ }
+
if (m_audioRenderers.contains(trackID))
return [m_audioRenderers.get(trackID) isReadyForMoreMediaData];
@@ -1049,10 +1053,8 @@
[m_displayLayer stopRequestingMediaData];
} else if (m_audioRenderers.contains(trackID))
[m_audioRenderers.get(trackID) stopRequestingMediaData];
- else {
- ASSERT_NOT_REACHED();
+ else
return;
- }
if (m_client)
m_client->sourceBufferPrivateDidBecomeReadyForMoreSamples(AtomicString::number(trackID));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
