[webkit-changes] [239857] trunk/Source/WebCore

Thu, 10 Jan 2019 20:48:34 -0800

Title: [239857] trunk/Source/WebCore
Revision
239857
Author
jer.no...@apple.com
Date
2019-01-10 20:46:23 -0800 (Thu, 10 Jan 2019)

Log Message

<video> elements do not enter 'paused' state when playing to end over AirPlay
https://bugs.webkit.org/show_bug.cgi?id=193295
<rdar://problem/46708670>

Reviewed by Eric Carlson.

Adopt the -[AVPlayer timeControlStatus] API, which reports whether the AVPlayer is paused, playing, or blocked waiting
for more data before playing. AirPlay devices report this state back from the remote device, and this allows the
MediaPlayerPrivateAVFoundationObjC to differentiate between user-generated pauses and simple stalling.

Adopting this API allows us to remove the heuristic from rateChanged() which inteprets a rate change when the
readyState > HAVE_ENOUGH as an intentional pause.

Drive-by fix: MediaPlayerPrivateAVFoundation had some code to delay calling platformPlay()
until the first frame became available. But this code was entirely undermined by the previous
behavior of setRate(). Fixing setRate()/setRateDouble() to only start playback if playback was
actually requested started making this code work for the first time, and broke some API tests.
Thus, we're removing this previously dead code.

* platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
(WebCore::MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation):
(WebCore::MediaPlayerPrivateAVFoundation::play):
(WebCore::MediaPlayerPrivateAVFoundation::pause):
(WebCore::MediaPlayerPrivateAVFoundation::rateChanged):
(WebCore::MediaPlayerPrivateAVFoundation::updateStates):
* platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::cancelLoad):
(WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer):
(WebCore::MediaPlayerPrivateAVFoundationObjC::didEnd):
(WebCore::MediaPlayerPrivateAVFoundationObjC::platformPlay):
(WebCore::MediaPlayerPrivateAVFoundationObjC::platformPause):
(WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime):
(WebCore::MediaPlayerPrivateAVFoundationObjC::setRateDouble):
(WebCore::MediaPlayerPrivateAVFoundationObjC::setPlayerRate):
(WebCore::MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange):
(WebCore::MediaPlayerPrivateAVFoundationObjC::setShouldObserveTimeControlStatus):
(-[WebCoreAVFMovieObserver observeValueForKeyPath:ofObject:change:context:]):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (239856 => 239857)


--- trunk/Source/WebCore/ChangeLog	2019-01-11 04:39:23 UTC (rev 239856)
+++ trunk/Source/WebCore/ChangeLog	2019-01-11 04:46:23 UTC (rev 239857)
@@ -1,3 +1,45 @@
+2019-01-10  Jer Noble  <jer.no...@apple.com>
+
+        <video> elements do not enter 'paused' state when playing to end over AirPlay
+        https://bugs.webkit.org/show_bug.cgi?id=193295
+        <rdar://problem/46708670>
+
+        Reviewed by Eric Carlson.
+
+        Adopt the -[AVPlayer timeControlStatus] API, which reports whether the AVPlayer is paused, playing, or blocked waiting
+        for more data before playing. AirPlay devices report this state back from the remote device, and this allows the
+        MediaPlayerPrivateAVFoundationObjC to differentiate between user-generated pauses and simple stalling.
+
+        Adopting this API allows us to remove the heuristic from rateChanged() which inteprets a rate change when the
+        readyState > HAVE_ENOUGH as an intentional pause.
+
+        Drive-by fix: MediaPlayerPrivateAVFoundation had some code to delay calling platformPlay()
+        until the first frame became available. But this code was entirely undermined by the previous
+        behavior of setRate(). Fixing setRate()/setRateDouble() to only start playback if playback was
+        actually requested started making this code work for the first time, and broke some API tests.
+        Thus, we're removing this previously dead code.
+
+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
+        (WebCore::MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation):
+        (WebCore::MediaPlayerPrivateAVFoundation::play):
+        (WebCore::MediaPlayerPrivateAVFoundation::pause):
+        (WebCore::MediaPlayerPrivateAVFoundation::rateChanged):
+        (WebCore::MediaPlayerPrivateAVFoundation::updateStates):
+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::cancelLoad):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::didEnd):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::platformPlay):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::platformPause):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::setRateDouble):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::setPlayerRate):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::setShouldObserveTimeControlStatus):
+        (-[WebCoreAVFMovieObserver observeValueForKeyPath:ofObject:change:context:]):
+
 2019-01-10  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Fix the build after r239844

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp (239856 => 239857)


--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp	2019-01-11 04:39:23 UTC (rev 239856)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp	2019-01-11 04:46:23 UTC (rev 239857)
@@ -79,7 +79,6 @@
     , m_cachedHasCaptions(false)
     , m_ignoreLoadStateChanges(false)
     , m_haveReportedFirstVideoFrame(false)
-    , m_playWhenFramesAvailable(false)
     , m_inbandTrackConfigurationPending(false)
     , m_characteristicsChanged(false)
     , m_shouldMaintainAspectRatio(true)
@@ -216,21 +215,12 @@
 void MediaPlayerPrivateAVFoundation::play()
 {
     ALWAYS_LOG(LOGIDENTIFIER);
-
-    // If the file has video, don't request playback until the first frame of video is ready to display
-    // or the audio may start playing before we can render video.
-    if (!m_cachedHasVideo || hasAvailableVideoFrame())
-        platformPlay();
-    else {
-        INFO_LOG(LOGIDENTIFIER, "waiting for first video frame");
-        m_playWhenFramesAvailable = true;
-    }
+    platformPlay();
 }
 
 void MediaPlayerPrivateAVFoundation::pause()
 {
     ALWAYS_LOG(LOGIDENTIFIER);
-    m_playWhenFramesAvailable = false;
     platformPause();
 }
 
@@ -576,11 +566,6 @@
 
     setNetworkState(newNetworkState);
     setReadyState(newReadyState);
-
-    if (m_playWhenFramesAvailable && hasAvailableVideoFrame()) {
-        m_playWhenFramesAvailable = false;
-        platformPlay();
-    }
 }
 
 void MediaPlayerPrivateAVFoundation::setSize(const IntSize&) 
@@ -623,17 +608,6 @@
 
 void MediaPlayerPrivateAVFoundation::rateChanged()
 {
-
-#if ENABLE(WIRELESS_PLAYBACK_TARGET) && PLATFORM(IOS_FAMILY)
-    if (isCurrentPlaybackTargetWireless() && playerItemStatus() >= MediaPlayerAVPlayerItemStatusPlaybackBufferFull) {
-        double rate = this->rate();
-        if (rate != requestedRate()) {
-            m_player->handlePlaybackCommand(rate ? PlatformMediaSession::PlayCommand : PlatformMediaSession::PauseCommand);
-            return;
-        }
-    }
-#endif
-
     m_player->rateChanged();
 }

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h (239856 => 239857)


--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h	2019-01-11 04:39:23 UTC (rev 239856)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h	2019-01-11 04:46:23 UTC (rev 239857)
@@ -369,7 +369,6 @@
     bool m_cachedHasCaptions;
     bool m_ignoreLoadStateChanges;
     bool m_haveReportedFirstVideoFrame;
-    bool m_playWhenFramesAvailable;
     bool m_inbandTrackConfigurationPending;
     bool m_characteristicsChanged;
     bool m_shouldMaintainAspectRatio;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (239856 => 239857)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h	2019-01-11 04:39:23 UTC (rev 239856)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h	2019-01-11 04:46:23 UTC (rev 239857)
@@ -80,6 +80,7 @@
 
     void setAsset(RetainPtr<id>&&);
     void tracksChanged() override;
+    void didEnd() override;
 
 #if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP)
     RetainPtr<AVPlayerItem> playerItem() const { return m_avPlayerItem; }
@@ -113,6 +114,7 @@
     void presentationSizeDidChange(FloatSize);
     void durationDidChange(const MediaTime&);
     void rateDidChange(double);
+    void timeControlStatusDidChange(int);
     void metadataDidArrive(const RetainPtr<NSArray>&, const MediaTime&);
     void firstFrameAvailableDidChange(bool);
     void trackEnabledDidChange(bool);
@@ -181,6 +183,7 @@
     void setVideoFullscreenGravity(MediaPlayer::VideoGravity) override;
     void setVideoFullscreenMode(MediaPlayer::VideoFullscreenMode) override;
     void videoFullscreenStandbyChanged() override;
+    void setPlayerRate(double);
 
 #if PLATFORM(IOS_FAMILY)
     NSArray *timedMetadata() const override;
@@ -333,6 +336,7 @@
     AVPlayer *objCAVFoundationAVPlayer() const final { return m_avPlayer.get(); }
 
     bool performTaskAtMediaTime(WTF::Function<void()>&&, MediaTime) final;
+    void setShouldObserveTimeControlStatus(bool);
 
     WeakPtrFactory<MediaPlayerPrivateAVFoundationObjC> m_weakPtrFactory;
     RetainPtr<AVURLAsset> m_avAsset;
@@ -415,6 +419,9 @@
     MediaTime m_cachedDuration;
     RefPtr<SharedBuffer> m_keyID;
     double m_cachedRate;
+    bool m_requestedPlaying { false };
+    double m_requestedRate { 1.0 };
+    int m_cachedTimeControlStatus { 0 };
     mutable long long m_cachedTotalBytes;
     unsigned m_pendingStatusChanges;
     int m_cachedItemStatus;
@@ -428,6 +435,7 @@
     bool m_cachedCanPlayFastForward;
     bool m_cachedCanPlayFastReverse;
     bool m_muted { false };
+    bool m_shouldObserveTimeControlStatus { false };
     mutable Optional<bool> m_tracksArePlayable;
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
     mutable bool m_allowsWirelessVideoPlayback;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (239856 => 239857)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2019-01-11 04:39:23 UTC (rev 239856)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm	2019-01-11 04:46:23 UTC (rev 239857)
@@ -587,6 +587,8 @@
         for (NSString *keyName in playerKVOProperties())
             [m_avPlayer.get() removeObserver:m_objcObserver.get() forKeyPath:keyName];
 
+        setShouldObserveTimeControlStatus(false);
+
         [m_avPlayer replaceCurrentItemWithPlayerItem:nil];
         m_avPlayer = nil;
     }
@@ -1011,6 +1013,8 @@
     for (NSString *keyName in playerKVOProperties())
         [m_avPlayer.get() addObserver:m_objcObserver.get() forKeyPath:keyName options:NSKeyValueObservingOptionNew context:(void *)MediaPlayerAVFoundationObservationContextPlayer];
 
+    setShouldObserveTimeControlStatus(true);
+
 #if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP) && HAVE(AVFOUNDATION_LEGIBLE_OUTPUT_SUPPORT)
     [m_avPlayer.get() setAppliesMediaSelectionCriteriaAutomatically:NO];
 #endif
@@ -1265,6 +1269,12 @@
 }
 #endif
 
+void MediaPlayerPrivateAVFoundationObjC::didEnd()
+{
+    m_requestedPlaying = false;
+    MediaPlayerPrivateAVFoundation::didEnd();
+}
+
 void MediaPlayerPrivateAVFoundationObjC::platformSetVisible(bool isVisible)
 {
     [CATransaction begin];
@@ -1280,10 +1290,8 @@
     if (!metaDataAvailable())
         return;
 
-    setDelayCallbacks(true);
-    m_cachedRate = requestedRate();
-    [m_avPlayer.get() setRate:requestedRate()];
-    setDelayCallbacks(false);
+    m_requestedPlaying = true;
+    setPlayerRate(m_requestedRate);
 }
 
 void MediaPlayerPrivateAVFoundationObjC::platformPause()
@@ -1292,10 +1300,8 @@
     if (!metaDataAvailable())
         return;
 
-    setDelayCallbacks(true);
-    m_cachedRate = 0;
-    [m_avPlayer.get() setRate:0];
-    setDelayCallbacks(false);
+    m_requestedPlaying = false;
+    setPlayerRate(0);
 }
 
 MediaTime MediaPlayerPrivateAVFoundationObjC::platformDuration() const
@@ -1353,6 +1359,7 @@
     
     auto weakThis = makeWeakPtr(*this);
 
+    setShouldObserveTimeControlStatus(false);
     [m_avPlayerItem.get() seekToTime:cmTime toleranceBefore:cmBefore toleranceAfter:cmAfter completionHandler:^(BOOL finished) {
         callOnMainThread([weakThis, finished] {
             auto _this = weakThis.get();
@@ -1359,6 +1366,7 @@
             if (!_this)
                 return;
 
+            _this->setShouldObserveTimeControlStatus(true);
             _this->seekCompleted(finished);
         });
     }];
@@ -1406,9 +1414,19 @@
 
 void MediaPlayerPrivateAVFoundationObjC::setRateDouble(double rate)
 {
+    m_requestedRate = rate;
+    if (m_requestedPlaying)
+        setPlayerRate(rate);
+}
+
+void MediaPlayerPrivateAVFoundationObjC::setPlayerRate(double rate)
+{
     setDelayCallbacks(true);
     m_cachedRate = rate;
-    [m_avPlayer.get() setRate:rate];
+    setShouldObserveTimeControlStatus(false);
+    [m_avPlayer setRate:rate];
+    m_cachedTimeControlStatus = [m_avPlayer timeControlStatus];
+    setShouldObserveTimeControlStatus(true);
     setDelayCallbacks(false);
 }
 
@@ -3234,8 +3252,28 @@
     rateChanged();
 }
 
+void MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange(int timeControlStatus)
+{
+    if (m_cachedTimeControlStatus == timeControlStatus)
+        return;
+
+    if (!m_shouldObserveTimeControlStatus)
+        return;
+
+    m_cachedTimeControlStatus = timeControlStatus;
+
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
+    if (!isCurrentPlaybackTargetWireless())
+        return;
 
+    bool playerIsPlaying = m_cachedTimeControlStatus != AVPlayerTimeControlStatusPaused;
+    if (playerIsPlaying != m_requestedPlaying)
+        player()->handlePlaybackCommand(playerIsPlaying ? PlatformMediaSession::PlayCommand : PlatformMediaSession::PauseCommand);
+#endif
+}
+
+#if ENABLE(WIRELESS_PLAYBACK_TARGET)
+
 void MediaPlayerPrivateAVFoundationObjC::playbackTargetIsWirelessDidChange()
 {
     playbackTargetIsWirelessChanged();
@@ -3310,6 +3348,18 @@
     return true;
 }
 
+void MediaPlayerPrivateAVFoundationObjC::setShouldObserveTimeControlStatus(bool shouldObserve)
+{
+    if (shouldObserve == m_shouldObserveTimeControlStatus)
+        return;
+
+    m_shouldObserveTimeControlStatus = shouldObserve;
+    if (shouldObserve)
+        [m_avPlayer addObserver:m_objcObserver.get() forKeyPath:@"timeControlStatus" options:NSKeyValueObservingOptionNew context:(void *)MediaPlayerAVFoundationObservationContextPlayer];
+    else
+        [m_avPlayer removeObserver:m_objcObserver.get() forKeyPath:@"timeControlStatus"];
+}
+
 NSArray* assetMetadataKeyNames()
 {
     static NSArray* keys = [[NSArray alloc] initWithObjects:
@@ -3473,6 +3523,8 @@
             // A value changed for an AVPlayer.
             if ([keyPath isEqualToString:@"rate"])
                 player->rateDidChange([newValue doubleValue]);
+            else if ([keyPath isEqualToString:@"timeControlStatus"])
+                player->timeControlStatusDidChange([newValue intValue]);
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
             else if ([keyPath isEqualToString:@"externalPlaybackActive"] || [keyPath isEqualToString:@"allowsExternalPlayback"])
                 player->playbackTargetIsWirelessDidChange();

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to