Diff
Modified: trunk/Source/WebCore/ChangeLog (262315 => 262316)
--- trunk/Source/WebCore/ChangeLog 2020-05-29 20:28:30 UTC (rev 262315)
+++ trunk/Source/WebCore/ChangeLog 2020-05-29 21:06:04 UTC (rev 262316)
@@ -1,3 +1,16 @@
+2020-05-29 Jacob Uphoff <jacob_uph...@apple.com>
+
+ Unreviewed, reverting r262289.
+
+ This commit caused a test to crash internally
+
+ Reverted changeset:
+
+ "MediaPlayerPrivateMediaStreamAVFObjC should enqueue samples
+ in a background thread"
+ https://bugs.webkit.org/show_bug.cgi?id=212073
+ https://trac.webkit.org/changeset/262289
+
2020-05-27 Darin Adler <da...@apple.com>
Remove things from FeatureDefines.xcconfig that are covered by PlatformEnableCocoa.h
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.h (262315 => 262316)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.h 2020-05-29 20:28:30 UTC (rev 262315)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.h 2020-05-29 21:06:04 UTC (rev 262316)
@@ -31,7 +31,6 @@
#include <wtf/Deque.h>
#include <wtf/Forward.h>
#include <wtf/RetainPtr.h>
-#include <wtf/WorkQueue.h>
OBJC_CLASS AVSampleBufferDisplayLayer;
OBJC_CLASS WebAVSampleBufferStatusChangeListener;
@@ -38,7 +37,7 @@
namespace WebCore {
-class WEBCORE_EXPORT LocalSampleBufferDisplayLayer final : public SampleBufferDisplayLayer, public CanMakeWeakPtr<LocalSampleBufferDisplayLayer, WeakPtrFactoryInitialization::Eager> {
+class WEBCORE_EXPORT LocalSampleBufferDisplayLayer final : public SampleBufferDisplayLayer, public CanMakeWeakPtr<LocalSampleBufferDisplayLayer> {
WTF_MAKE_FAST_ALLOCATED;
public:
static std::unique_ptr<LocalSampleBufferDisplayLayer> create(Client&);
@@ -79,7 +78,6 @@
void removeOldSamplesFromPendingQueue();
void addSampleToPendingQueue(MediaSample&);
void requestNotificationWhenReadyForVideoData();
- void enqueueSampleBuffer(MediaSample&);
private:
RetainPtr<WebAVSampleBufferStatusChangeListener> m_statusChangeListener;
@@ -86,10 +84,7 @@
RetainPtr<AVSampleBufferDisplayLayer> m_sampleBufferDisplayLayer;
RetainPtr<PlatformLayer> m_rootLayer;
RenderPolicy m_renderPolicy { RenderPolicy::TimingInfo };
-
- RefPtr<WorkQueue> m_processingQueue;
- // Only accessed through m_processingQueue or if m_processingQueue is null.
using PendingSampleQueue = Deque<Ref<MediaSample>>;
PendingSampleQueue m_pendingVideoSampleQueue;
};
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm (262315 => 262316)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm 2020-05-29 20:28:30 UTC (rev 262315)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm 2020-05-29 21:06:04 UTC (rev 262316)
@@ -105,24 +105,33 @@
UNUSED_PARAM(context);
UNUSED_PARAM(keyPath);
UNUSED_PARAM(change);
+ ASSERT(_parent);
- if (![object isKindOfClass:PAL::getAVSampleBufferDisplayLayerClass()])
+ if (!_parent)
return;
- if ([keyPath isEqualToString:@"status"]) {
- callOnMainThread([protectedSelf = RetainPtr<WebAVSampleBufferStatusChangeListener>(self)] {
- if (protectedSelf->_parent)
+ if ([object isKindOfClass:PAL::getAVSampleBufferDisplayLayerClass()]) {
+ ASSERT(object == _parent->displayLayer());
+
+ if ([keyPath isEqualToString:@"status"]) {
+ callOnMainThread([protectedSelf = RetainPtr<WebAVSampleBufferStatusChangeListener>(self)] {
+ if (!protectedSelf->_parent)
+ return;
+
protectedSelf->_parent->layerStatusDidChange();
- });
- return;
- }
+ });
+ return;
+ }
- if ([keyPath isEqualToString:@"error"]) {
- callOnMainThread([protectedSelf = RetainPtr<WebAVSampleBufferStatusChangeListener>(self)] {
- if (protectedSelf->_parent)
+ if ([keyPath isEqualToString:@"error"]) {
+ callOnMainThread([protectedSelf = RetainPtr<WebAVSampleBufferStatusChangeListener>(self)] {
+ if (!protectedSelf->_parent)
+ return;
+
protectedSelf->_parent->layerErrorDidChange();
- });
- return;
+ });
+ return;
+ }
}
}
@end
@@ -151,7 +160,6 @@
: SampleBufferDisplayLayer(client)
, m_statusChangeListener(adoptNS([[WebAVSampleBufferStatusChangeListener alloc] initWithParent:this]))
, m_sampleBufferDisplayLayer(WTFMove(sampleBufferDisplayLayer))
- , m_processingQueue(WorkQueue::create("LocalSampleBufferDisplayLayer queue"))
{
}
@@ -183,8 +191,6 @@
LocalSampleBufferDisplayLayer::~LocalSampleBufferDisplayLayer()
{
- m_processingQueue = nullptr;
-
[m_statusChangeListener stop];
m_pendingVideoSampleQueue.clear();
@@ -275,16 +281,12 @@
void LocalSampleBufferDisplayLayer::flush()
{
- m_processingQueue->dispatch([this] {
- [m_sampleBufferDisplayLayer flush];
- });
+ [m_sampleBufferDisplayLayer flush];
}
void LocalSampleBufferDisplayLayer::flushAndRemoveImage()
{
- m_processingQueue->dispatch([this] {
- [m_sampleBufferDisplayLayer flushAndRemoveImage];
- });
+ [m_sampleBufferDisplayLayer flushAndRemoveImage];
}
static const double rendererLatency = 0.02;
@@ -291,20 +293,12 @@
void LocalSampleBufferDisplayLayer::enqueueSample(MediaSample& sample)
{
- m_processingQueue->dispatch([this, sample = makeRef(sample)] {
- if (![m_sampleBufferDisplayLayer isReadyForMoreMediaData]) {
- addSampleToPendingQueue(sample);
- requestNotificationWhenReadyForVideoData();
- return;
- }
- enqueueSampleBuffer(sample);
- });
-}
+ if (![m_sampleBufferDisplayLayer isReadyForMoreMediaData]) {
+ addSampleToPendingQueue(sample);
+ requestNotificationWhenReadyForVideoData();
+ return;
+ }
-void LocalSampleBufferDisplayLayer::enqueueSampleBuffer(MediaSample& sample)
-{
- ASSERT(!isMainThread());
-
auto sampleToEnqueue = sample.platformSample().sample.cmSampleBuffer;
auto now = MediaTime::createWithDouble(MonotonicTime::now().secondsSinceEpoch().value() + rendererLatency);
@@ -321,8 +315,6 @@
void LocalSampleBufferDisplayLayer::removeOldSamplesFromPendingQueue()
{
- ASSERT(!isMainThread());
-
if (m_pendingVideoSampleQueue.isEmpty())
return;
@@ -342,8 +334,6 @@
void LocalSampleBufferDisplayLayer::addSampleToPendingQueue(MediaSample& sample)
{
- ASSERT(!isMainThread());
-
removeOldSamplesFromPendingQueue();
m_pendingVideoSampleQueue.append(sample);
}
@@ -350,15 +340,13 @@
void LocalSampleBufferDisplayLayer::clearEnqueuedSamples()
{
- m_processingQueue->dispatch([this] {
- m_pendingVideoSampleQueue.clear();
- });
+ m_pendingVideoSampleQueue.clear();
}
void LocalSampleBufferDisplayLayer::requestNotificationWhenReadyForVideoData()
{
auto weakThis = makeWeakPtr(*this);
- [m_sampleBufferDisplayLayer requestMediaDataWhenReadyOnQueue:m_processingQueue->dispatchQueue() usingBlock:^{
+ [m_sampleBufferDisplayLayer requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^{
if (!weakThis)
return;
@@ -370,7 +358,8 @@
return;
}
- enqueueSampleBuffer(m_pendingVideoSampleQueue.takeFirst().get());
+ auto sample = m_pendingVideoSampleQueue.takeFirst();
+ enqueueSample(sample.get());
}
}];
}
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h (262315 => 262316)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h 2020-05-29 20:28:30 UTC (rev 262315)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h 2020-05-29 21:06:04 UTC (rev 262316)
@@ -137,7 +137,6 @@
void flushRenderers();
- void processNewVideoSample(MediaSample&, bool hasChangedOrientation);
void enqueueVideoSample(MediaSample&);
void requestNotificationWhenReadyForVideoData();
@@ -213,6 +212,8 @@
AudioSourceProvider* audioSourceProvider() final;
+ CGAffineTransform videoTransformationMatrix(MediaSample&, bool forceUpdate = false);
+
void applicationDidBecomeActive() final;
bool hideRootLayer() const { return (!activeVideoTrack() || m_waitingForFirstImage) && m_displayMode != PaintItBlack; }
@@ -245,17 +246,9 @@
float m_volume { 1 };
DisplayMode m_displayMode { None };
PlaybackState m_playbackState { PlaybackState::None };
- Optional<CGAffineTransform> m_videoTransform;
-
- // Used on both main thread and sample thread.
+ MediaSample::VideoRotation m_videoRotation { MediaSample::VideoRotation::None };
+ CGAffineTransform m_videoTransform;
std::unique_ptr<SampleBufferDisplayLayer> m_sampleBufferDisplayLayer;
- Lock m_sampleBufferDisplayLayerLock;
- bool m_shouldUpdateDisplayLayer { true };
- // Written on main thread, read on sample thread.
- bool m_canEnqueueDisplayLayer { false };
- // Used on sample thread.
- MediaSample::VideoRotation m_videoRotation { MediaSample::VideoRotation::None };
- bool m_videoMirrored { false };
Ref<const Logger> m_logger;
const void* m_logIdentifier;
@@ -266,11 +259,13 @@
RetainPtr<WebRootSampleBufferBoundsChangeListener> m_boundsChangeListener;
+ bool m_videoMirrored { false };
bool m_playing { false };
bool m_muted { false };
bool m_ended { false };
bool m_hasEverEnqueuedVideoFrame { false };
bool m_pendingSelectedTrackCheck { false };
+ bool m_transformIsValid { false };
bool m_visible { false };
bool m_haveSeenMetadata { false };
bool m_waitingForFirstImage { false };
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm (262315 => 262316)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm 2020-05-29 20:28:30 UTC (rev 262315)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm 2020-05-29 21:06:04 UTC (rev 262316)
@@ -41,7 +41,6 @@
#import <pal/avfoundation/MediaTimeAVFoundation.h>
#import <pal/spi/cocoa/AVFoundationSPI.h>
#import <pal/system/Clock.h>
-#import <wtf/Lock.h>
#import <wtf/MainThread.h>
#import <wtf/NeverDestroyed.h>
@@ -140,7 +139,7 @@
, m_videoLayerManager(makeUnique<VideoLayerManagerObjC>(m_logger, m_logIdentifier))
{
INFO_LOG(LOGIDENTIFIER);
- // MediaPlayerPrivateMediaStreamAVFObjC::processNewVideoSample expects a weak pointer to be created in the constructor.
+ // MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable expects a weak pointer to be created in the constructor.
m_boundsChangeListener = adoptNS([[WebRootSampleBufferBoundsChangeListener alloc] initWithCallback:[this, weakThis = makeWeakPtr(this)] {
if (!weakThis)
return;
@@ -228,8 +227,11 @@
#pragma mark -
#pragma mark AVSampleBuffer Methods
-static inline CGAffineTransform videoTransformationMatrix(MediaSample& sample)
+CGAffineTransform MediaPlayerPrivateMediaStreamAVFObjC::videoTransformationMatrix(MediaSample& sample, bool forceUpdate)
{
+ if (!forceUpdate && m_transformIsValid)
+ return m_videoTransform;
+
CMSampleBufferRef sampleBuffer = sample.platformSample().sample.cmSampleBuffer;
CVPixelBufferRef pixelBuffer = static_cast<CVPixelBufferRef>(CMSampleBufferGetImageBuffer(sampleBuffer));
size_t width = CVPixelBufferGetWidth(pixelBuffer);
@@ -237,28 +239,19 @@
if (!width || !height)
return CGAffineTransformIdentity;
- auto videoTransform = CGAffineTransformMakeRotation(static_cast<int>(sample.videoRotation()) * M_PI / 180);
+ ASSERT(m_videoRotation >= MediaSample::VideoRotation::None);
+ ASSERT(m_videoRotation <= MediaSample::VideoRotation::Left);
+
+ m_videoTransform = CGAffineTransformMakeRotation(static_cast<int>(m_videoRotation) * M_PI / 180);
if (sample.videoMirrored())
- videoTransform = CGAffineTransformScale(videoTransform, -1, 1);
+ m_videoTransform = CGAffineTransformScale(m_videoTransform, -1, 1);
- return videoTransform;
+ m_transformIsValid = true;
+ return m_videoTransform;
}
-void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& sample)
-{
- processNewVideoSample(sample, sample.videoRotation() != m_videoRotation || sample.videoMirrored() != m_videoMirrored);
- enqueueVideoSample(sample);
-}
-
void MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample(MediaSample& sample)
{
- if (!m_canEnqueueDisplayLayer)
- return;
-
- auto locker = tryHoldLock(m_sampleBufferDisplayLayerLock);
- if (!locker)
- return;
-
if (!m_sampleBufferDisplayLayer || m_sampleBufferDisplayLayer->didFail())
return;
@@ -265,23 +258,18 @@
if (sample.videoRotation() != m_videoRotation || sample.videoMirrored() != m_videoMirrored) {
m_videoRotation = sample.videoRotation();
m_videoMirrored = sample.videoMirrored();
- m_sampleBufferDisplayLayer->updateAffineTransform(videoTransformationMatrix(sample));
- m_shouldUpdateDisplayLayer = true;
+ m_sampleBufferDisplayLayer->updateAffineTransform(videoTransformationMatrix(sample, true));
+ updateDisplayLayer();
}
- if (m_shouldUpdateDisplayLayer) {
- m_sampleBufferDisplayLayer->updateBoundsAndPosition(m_sampleBufferDisplayLayer->rootLayer().bounds, m_videoRotation);
- m_shouldUpdateDisplayLayer = false;
- }
-
m_sampleBufferDisplayLayer->enqueueSample(sample);
}
-void MediaPlayerPrivateMediaStreamAVFObjC::processNewVideoSample(MediaSample& sample, bool hasChangedOrientation)
+void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& sample)
{
if (!isMainThread()) {
- callOnMainThread([weakThis = makeWeakPtr(this), sample = makeRef(sample), hasChangedOrientation]() mutable {
+ callOnMainThread([weakThis = makeWeakPtr(this), sample = makeRef(sample)]() mutable {
if (weakThis)
- weakThis->processNewVideoSample(sample.get(), hasChangedOrientation);
+ weakThis->videoSampleAvailable(sample.get());
});
return;
}
@@ -289,9 +277,6 @@
if (!m_activeVideoTrack)
return;
- if (hasChangedOrientation)
- m_videoTransform = { };
-
if (!m_imagePainter.mediaSample || m_displayMode != PausedImage) {
m_imagePainter.mediaSample = &sample;
m_imagePainter.cgImage = nullptr;
@@ -302,6 +287,8 @@
if (m_displayMode != LivePreview && !m_waitingForFirstImage)
return;
+ enqueueVideoSample(sample);
+
if (!m_hasEverEnqueuedVideoFrame) {
m_hasEverEnqueuedVideoFrame = true;
m_player->firstVideoFrameAvailable();
@@ -348,7 +335,6 @@
if (!activeVideoTrack || !activeVideoTrack->enabled())
return;
- m_canEnqueueDisplayLayer = false;
m_sampleBufferDisplayLayer = SampleBufferDisplayLayer::create(*this);
ERROR_LOG_IF(!m_sampleBufferDisplayLayer, LOGIDENTIFIER, "Creating the SampleBufferDisplayLayer failed.");
if (!m_sampleBufferDisplayLayer)
@@ -365,21 +351,16 @@
return;
}
updateRenderingMode();
- m_shouldUpdateDisplayLayer = true;
+ updateDisplayLayer();
m_videoLayerManager->setVideoLayer(m_sampleBufferDisplayLayer->rootLayer(), size);
[m_boundsChangeListener begin:m_sampleBufferDisplayLayer->rootLayer()];
-
- m_canEnqueueDisplayLayer = true;
});
}
void MediaPlayerPrivateMediaStreamAVFObjC::destroyLayers()
{
- m_canEnqueueDisplayLayer = false;
-
- auto locker = holdLock(m_sampleBufferDisplayLayerLock);
if (m_sampleBufferDisplayLayer)
m_sampleBufferDisplayLayer = nullptr;
@@ -680,7 +661,7 @@
return;
scheduleDeferredTask([this] {
- m_videoTransform = { };
+ m_transformIsValid = false;
if (m_player)
m_player->renderingModeChanged();
});
@@ -963,9 +944,7 @@
auto image = m_imagePainter.cgImage.get();
FloatRect imageRect(0, 0, CGImageGetWidth(image), CGImageGetHeight(image));
- if (!m_videoTransform)
- m_videoTransform = videoTransformationMatrix(*m_imagePainter.mediaSample);
- AffineTransform videoTransform = *m_videoTransform;
+ AffineTransform videoTransform = videoTransformationMatrix(*m_imagePainter.mediaSample);
FloatRect transformedDestRect = videoTransform.inverse().valueOr(AffineTransform()).mapRect(destRect);
context.concatCTM(videoTransform);
context.drawNativeImage(image, imageRect.size(), transformedDestRect, imageRect);
@@ -1031,9 +1010,17 @@
pixelBufferConformer = nullptr;
}
+void MediaPlayerPrivateMediaStreamAVFObjC::updateDisplayLayer()
+{
+ if (!m_sampleBufferDisplayLayer)
+ return;
+
+ m_sampleBufferDisplayLayer->updateBoundsAndPosition(m_sampleBufferDisplayLayer->rootLayer().bounds, m_videoRotation);
+}
+
void MediaPlayerPrivateMediaStreamAVFObjC::rootLayerBoundsDidChange()
{
- m_shouldUpdateDisplayLayer = true;
+ updateDisplayLayer();
}
WTFLogChannel& MediaPlayerPrivateMediaStreamAVFObjC::logChannel() const
