<video> elements do not enter 'paused' state when playing to end over AirPlay
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2019 04:46:23 +0000 (04:46 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2019 04:46:23 +0000 (04:46 +0000)
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:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239857 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

index fb8d9c2..f336428 100644 (file)
@@ -1,3 +1,45 @@
+2019-01-10  Jer Noble  <jer.noble@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  <mmaxfield@apple.com>
 
         Fix the build after r239844
index 8bb0761..6ef3342 100644 (file)
@@ -79,7 +79,6 @@ MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation(MediaPlayer* play
     , 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::prepareToPlay()
 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 @@ void MediaPlayerPrivateAVFoundation::updateStates()
 
     setNetworkState(newNetworkState);
     setReadyState(newReadyState);
-
-    if (m_playWhenFramesAvailable && hasAvailableVideoFrame()) {
-        m_playWhenFramesAvailable = false;
-        platformPlay();
-    }
 }
 
 void MediaPlayerPrivateAVFoundation::setSize(const IntSize&) 
@@ -623,17 +608,6 @@ void MediaPlayerPrivateAVFoundation::metadataLoaded()
 
 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();
 }
 
index 3fbcdaf..7a90365 100644 (file)
@@ -369,7 +369,6 @@ private:
     bool m_cachedHasCaptions;
     bool m_ignoreLoadStateChanges;
     bool m_haveReportedFirstVideoFrame;
-    bool m_playWhenFramesAvailable;
     bool m_inbandTrackConfigurationPending;
     bool m_characteristicsChanged;
     bool m_shouldMaintainAspectRatio;
index e5bd79d..c7c9303 100644 (file)
@@ -80,6 +80,7 @@ public:
 
     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 @@ public:
     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 @@ private:
     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 @@ private:
     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 @@ private:
     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 @@ private:
     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;
index c1f38e3..2241db1 100644 (file)
@@ -587,6 +587,8 @@ void MediaPlayerPrivateAVFoundationObjC::cancelLoad()
         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 @@ void MediaPlayerPrivateAVFoundationObjC::createAVPlayer()
     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 @@ String MediaPlayerPrivateAVFoundationObjC::errorLog() const
 }
 #endif
 
+void MediaPlayerPrivateAVFoundationObjC::didEnd()
+{
+    m_requestedPlaying = false;
+    MediaPlayerPrivateAVFoundation::didEnd();
+}
+
 void MediaPlayerPrivateAVFoundationObjC::platformSetVisible(bool isVisible)
 {
     [CATransaction begin];
@@ -1280,10 +1290,8 @@ void MediaPlayerPrivateAVFoundationObjC::platformPlay()
     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 @@ void MediaPlayerPrivateAVFoundationObjC::platformPause()
     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,12 +1359,14 @@ void MediaPlayerPrivateAVFoundationObjC::seekToTime(const MediaTime& time, const
     
     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();
             if (!_this)
                 return;
 
+            _this->setShouldObserveTimeControlStatus(true);
             _this->seekCompleted(finished);
         });
     }];
@@ -1406,9 +1414,19 @@ void MediaPlayerPrivateAVFoundationObjC::setClosedCaptionsVisible(bool closedCap
 
 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,6 +3252,26 @@ void MediaPlayerPrivateAVFoundationObjC::rateDidChange(double rate)
     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()
@@ -3310,6 +3348,18 @@ bool MediaPlayerPrivateAVFoundationObjC::performTaskAtMediaTime(WTF::Function<vo
     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 @@ NSArray* playerKVOProperties()
             // 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();