Title: [257926] trunk/Source/WebCore
- Revision
- 257926
- Author
- eric.carl...@apple.com
- Date
- 2020-03-05 10:27:03 -0800 (Thu, 05 Mar 2020)
Log Message
Some media tests crash when run in the GPU process
https://bugs.webkit.org/show_bug.cgi?id=208611
<rdar://problem/60060320>
Reviewed by Jer Noble.
The AVPlayerItemOutputPullDelegate is not called on the main queue, so it can't
use a WeakPtr that is created on the main queue. Rather than having the ObjC
delegate object call back to MediaPlayerPrivateAVFoundationObjC to signal a
semaphore it owns, have the delegate object own the semaphore and expose it
so MediaPlayerPrivateAVFoundationObjC can use it, removing the need for the
WeakPtr completely.
No new tests, this fixes a crash in existing tests.
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
Don't allocate m_videoOutputDelegate, it isn't needed immediately.
(WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenLayer): Drive-by
optimization: don't update the current image when the fullscreen layer is set to NULL.
(WebCore::MediaPlayerPrivateAVFoundationObjC::createVideoOutput): Allocate m_videoOutputDelegate.
(WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
Wait on the deletage's semphore.
(-[WebCoreAVFPullDelegate outputMediaDataWillChange:]):
(WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange): Deleted.
(-[WebCoreAVFPullDelegate initWithPlayer:]): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (257925 => 257926)
--- trunk/Source/WebCore/ChangeLog 2020-03-05 18:16:22 UTC (rev 257925)
+++ trunk/Source/WebCore/ChangeLog 2020-03-05 18:27:03 UTC (rev 257926)
@@ -1,3 +1,33 @@
+2020-03-05 Eric Carlson <eric.carl...@apple.com>
+
+ Some media tests crash when run in the GPU process
+ https://bugs.webkit.org/show_bug.cgi?id=208611
+ <rdar://problem/60060320>
+
+ Reviewed by Jer Noble.
+
+ The AVPlayerItemOutputPullDelegate is not called on the main queue, so it can't
+ use a WeakPtr that is created on the main queue. Rather than having the ObjC
+ delegate object call back to MediaPlayerPrivateAVFoundationObjC to signal a
+ semaphore it owns, have the delegate object own the semaphore and expose it
+ so MediaPlayerPrivateAVFoundationObjC can use it, removing the need for the
+ WeakPtr completely.
+
+ No new tests, this fixes a crash in existing tests.
+
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
+ Don't allocate m_videoOutputDelegate, it isn't needed immediately.
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenLayer): Drive-by
+ optimization: don't update the current image when the fullscreen layer is set to NULL.
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::createVideoOutput): Allocate m_videoOutputDelegate.
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
+ Wait on the deletage's semphore.
+ (-[WebCoreAVFPullDelegate outputMediaDataWillChange:]):
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange): Deleted.
+ (-[WebCoreAVFPullDelegate initWithPlayer:]): Deleted.
+
2020-03-05 Simon Fraser <simon.fra...@apple.com>
Change ScrollingTreeMac.cpp to a .mm file
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (257925 => 257926)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2020-03-05 18:16:22 UTC (rev 257925)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2020-03-05 18:27:03 UTC (rev 257926)
@@ -126,8 +126,6 @@
void setBufferingPolicy(MediaPlayer::BufferingPolicy) override;
- void outputMediaDataWillChange(AVPlayerItemVideoOutput*);
-
#if ENABLE(WIRELESS_PLAYBACK_TARGET)
void playbackTargetIsWirelessDidChange();
#endif
@@ -358,7 +356,6 @@
RetainPtr<WebCoreAVFPullDelegate> m_videoOutputDelegate;
RetainPtr<CVPixelBufferRef> m_lastPixelBuffer;
RetainPtr<CGImageRef> m_lastImage;
- BinarySemaphore m_videoOutputSemaphore;
std::unique_ptr<ImageRotationSessionVT> m_imageRotationSession;
std::unique_ptr<VideoTextureCopierCV> m_videoTextureCopier;
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (257925 => 257926)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2020-03-05 18:16:22 UTC (rev 257925)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2020-03-05 18:27:03 UTC (rev 257926)
@@ -203,11 +203,12 @@
#endif
@interface WebCoreAVFPullDelegate : NSObject<AVPlayerItemOutputPullDelegate> {
- WeakPtr<MediaPlayerPrivateAVFoundationObjC> m_player;
+ BinarySemaphore m_semaphore;
}
-- (id)initWithPlayer:(WeakPtr<MediaPlayerPrivateAVFoundationObjC>&&)player;
- (void)outputMediaDataWillChange:(AVPlayerItemOutput *)sender;
- (void)outputSequenceWasFlushed:(AVPlayerItemOutput *)output;
+
+@property (nonatomic, readonly) BinarySemaphore& semaphore;
@end
namespace WebCore {
@@ -410,7 +411,6 @@
, m_objcObserver(adoptNS([[WebCoreAVFMovieObserver alloc] initWithPlayer:makeWeakPtr(*this)]))
, m_videoFrameHasDrawn(false)
, m_haveCheckedPlayability(false)
- , m_videoOutputDelegate(adoptNS([[WebCoreAVFPullDelegate alloc] initWithPlayer:makeWeakPtr(*this)]))
#if HAVE(AVFOUNDATION_LOADER_DELEGATE)
, m_loaderDelegate(adoptNS([[WebCoreAVFLoaderDelegate alloc] initWithPlayer:makeWeakPtr(*this)]))
#endif
@@ -1070,7 +1070,8 @@
void MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenLayer(PlatformLayer* videoFullscreenLayer, Function<void()>&& completionHandler)
{
- updateLastImage(UpdateType::UpdateSynchronously);
+ if (videoFullscreenLayer)
+ updateLastImage(UpdateType::UpdateSynchronously);
m_videoLayerManager->setVideoFullscreenLayer(videoFullscreenLayer, WTFMove(completionHandler), m_lastImage);
updateDisableExternalPlayback();
}
@@ -2129,6 +2130,7 @@
m_videoOutput = adoptNS([PAL::allocAVPlayerItemVideoOutputInstance() initWithPixelBufferAttributes:attributes]);
ASSERT(m_videoOutput);
+ m_videoOutputDelegate = adoptNS([[WebCoreAVFPullDelegate alloc] init]);
[m_videoOutput setDelegate:m_videoOutputDelegate.get() queue:globalPullDelegateQueue()];
[m_avPlayerItem.get() addOutput:m_videoOutput.get()];
@@ -2266,16 +2268,11 @@
[m_videoOutput requestNotificationOfMediaDataChangeWithAdvanceInterval:0];
// Wait for 1 second.
- bool satisfied = m_videoOutputSemaphore.waitFor(1_s);
+ bool satisfied = [m_videoOutputDelegate semaphore].waitFor(1_s);
if (!satisfied)
ERROR_LOG(LOGIDENTIFIER, "timed out");
}
-void MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange(AVPlayerItemVideoOutput *)
-{
- m_videoOutputSemaphore.signal();
-}
-
#if ENABLE(LEGACY_ENCRYPTED_MEDIA)
RetainPtr<AVAssetResourceLoadingRequest> MediaPlayerPrivateAVFoundationObjC::takeRequestForKeyURI(const String& keyURI)
@@ -3565,18 +3562,12 @@
@implementation WebCoreAVFPullDelegate
-- (id)initWithPlayer:(WeakPtr<MediaPlayerPrivateAVFoundationObjC>&&)player
-{
- self = [super init];
- if (self)
- m_player = WTFMove(player);
- return self;
-}
+@synthesize semaphore=m_semaphore;
- (void)outputMediaDataWillChange:(AVPlayerItemVideoOutput *)output
{
- if (m_player)
- m_player->outputMediaDataWillChange(output);
+ UNUSED_PARAM(output);
+ m_semaphore.signal();
}
- (void)outputSequenceWasFlushed:(AVPlayerItemVideoOutput *)output
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
