Attachment 358835 Details for Bug 193295 – Patch for landing

[patch] Patch for landing

bug-193295-20190110141819.patch (text/plain), 11.65 KB, created by Jer Noble on 2019-01-10 14:18:20 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2019-01-10 14:18:20 PST

Size: 11.65 KB

patch

obsolete

>Subversion Revision: 239617
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 92ff6fc0b6d7426f2bd769f7af87b4461ebc6862..af35551b4c11087f02ea8ed1bff78b6df74c65b9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+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.
>+
>+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
>+        (WebCore::MediaPlayerPrivateAVFoundation::rateChanged):
>+        * 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-04  Jer Noble  <jer.noble@apple.com>
> 
>         Web Content process main thread blocked beneath ImageDecoderAVFObjC::readSamples for many seconds on imgur.com
>diff --git a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>index 8bb0761900ba1818349dbabdd7f3eb26613223d7..3a0cf6baa2d6a9dd1ef610ac427a316e4f8d9b42 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>+++ b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>@@ -623,17 +623,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();
> }
> 
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>index e5bd79d10c53a5cbb88b2a47cd79e93e93f5b25b..c7c93030c80e0376e3f8df857f211f7f4b18bed2 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>@@ -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;
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>index c1f38e3bbcf059a392cb5543bc47dde266bc6329..383fdca47e5df7799bcb9cb66dba45e3b2dfec44 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>@@ -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);
>         });
>     }];
>@@ -1405,10 +1413,20 @@ 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,22 @@ void MediaPlayerPrivateAVFoundationObjC::rateDidChange(double rate)
>     rateChanged();
> }
> 
>+void MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange(int timeControlStatus)
>+{
>+    if (m_cachedTimeControlStatus == timeControlStatus)
>+        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 +3344,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 +3519,8 @@ - (void)observeValueForKeyPath:keyPath ofObject:(id)object change:(NSDictionary
>             // 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();

Actions: View | Formatted Diff | Diff

Attachments on bug 193295: 358737 | 358739 | 358744 | 358754 | 358835 | 358854