Attachment 358854 Details for Bug 193295 – Patch for landing

[patch] Patch for landing

bug-193295-20190110170717.patch (text/plain), 14.65 KB, created by Jer Noble on 2019-01-10 17:07:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2019-01-10 17:07:19 PST

Size: 14.65 KB

patch

obsolete

>Subversion Revision: 239617
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 92ff6fc0b6d7426f2bd769f7af87b4461ebc6862..90a99b194b6e2d02090c408e637848fa27e9014c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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-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..6ef334206724b5c142ae30d669fbad07ddf9fce0 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>+++ b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>@@ -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();
> }
> 
>diff --git a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>index 3fbcdafac7ae41d1b26b4a95b048e6b7242f0b0b..7a90365fe167afbbcd1cb2c48ee441afd8388e4e 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>+++ b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>@@ -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;
>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..2241db1522ac2c1b434a416031d23422e9c900ba 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,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 @@ - (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