Attachment 359384 Details for Bug 193499 – Patch for landing

[patch] Patch for landing

bug-193499-20190117093437.patch (text/plain), 7.96 KB, created by Jer Noble on 2019-01-17 09:34:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2019-01-17 09:34:37 PST

Size: 7.96 KB

patch

obsolete

>Subversion Revision: 240051
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 36d67f113d70ff50bc4d3990b47bf817e489f09f..b7c49cbbd5b3145e5c18dc2c1b06150e66e34fa0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-01-16  Jer Noble  <jer.noble@apple.com>
>+
>+        MediaPlayerPrivateAVFoundationObjC can return incorrect paused information
>+        https://bugs.webkit.org/show_bug.cgi?id=193499
>+
>+        Reviewed by Eric Carlson.
>+
>+        MediaPlayerPrivateAVFoundation uses rate() as an indicator of whether the player
>+        is paused or not. This is incorrect when playback is stalled waiting for more data.
>+        For MPPAVFObjC, use the timeControlStatus as a more accurate indicator of whether
>+        the player is playing.
>+
>+        Now that we have correct play state information, we can remove the handlePlaybackCommand()
>+        path when playing remotely for a more direct approach of notifying the HTMLMediaElement
>+        that the play state has changed.
>+
>+        Drive-by fix: Before throwing away the AVPlayer, clear its output context. This keeps
>+        remote devices from keeping the AVPlayer alive.
>+
>+        Drive-by fix #2: The NullMediaPlayer should always return "true" for paused(), not "false",
>+        since it can't possibly play anything.
>+
>+        * platform/graphics/MediaPlayer.cpp:
>+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
>+        (WebCore::MediaPlayerPrivateAVFoundation::paused const):
>+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
>+        (WebCore::MediaPlayerPrivateAVFoundation::platformPaused const):
>+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
>+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
>+        (WebCore::MediaPlayerPrivateAVFoundationObjC::cancelLoad):
>+        (WebCore::MediaPlayerPrivateAVFoundationObjC::platformPaused const):
>+        (WebCore::MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange):
>+
> 2019-01-14  Jer Noble  <jer.noble@apple.com>
> 
>         https://bugs.webkit.org/show_bug.cgi?id=193403
>diff --git a/Source/WebCore/PAL/pal/spi/mac/AVFoundationSPI.h b/Source/WebCore/PAL/pal/spi/mac/AVFoundationSPI.h
>index 17f8ef0194588d5629f652910ba2cdfe5c899b71..fb30cc863daf8bf379abb0d0419a8fdda3e805d5 100644
>--- a/Source/WebCore/PAL/pal/spi/mac/AVFoundationSPI.h
>+++ b/Source/WebCore/PAL/pal/spi/mac/AVFoundationSPI.h
>@@ -79,7 +79,7 @@ NS_ASSUME_NONNULL_BEGIN
> 
> #if !PLATFORM(IOS_FAMILY)
> @interface AVPlayer (AVPlayerExternalPlaybackSupportPrivate)
>-@property (nonatomic, retain) AVOutputContext *outputContext;
>+@property (nonatomic, retain, nullable) AVOutputContext *outputContext;
> @end
> #else
> typedef NS_ENUM(NSInteger, AVPlayerExternalPlaybackType) {
>diff --git a/Source/WebCore/platform/graphics/MediaPlayer.cpp b/Source/WebCore/platform/graphics/MediaPlayer.cpp
>index f1e5610b91e1d217c84611b56103126c0e9da57d..49c449a2660bb0074266cfdd9ac58c095e0079e0 100644
>--- a/Source/WebCore/platform/graphics/MediaPlayer.cpp
>+++ b/Source/WebCore/platform/graphics/MediaPlayer.cpp
>@@ -130,7 +130,7 @@ public:
> 
>     void setRateDouble(double) override { }
>     void setPreservesPitch(bool) override { }
>-    bool paused() const override { return false; }
>+    bool paused() const override { return true; }
> 
>     void setVolumeDouble(double) override { }
> 
>diff --git a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>index 6ef334206724b5c142ae30d669fbad07ddf9fce0..31030737bbfaccdca67f9c5d98f459b9a86d0db3 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>+++ b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp
>@@ -275,7 +275,7 @@ bool MediaPlayerPrivateAVFoundation::paused() const
>     if (!metaDataAvailable())
>         return true;
> 
>-    return rate() == 0;
>+    return platformPaused();
> }
> 
> bool MediaPlayerPrivateAVFoundation::seeking() const
>diff --git a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>index 7a90365fe167afbbcd1cb2c48ee441afd8388e4e..51a29bd18a94597c4883b447d963e8b3e0626fa1 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>+++ b/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h
>@@ -245,6 +245,7 @@ protected:
>     virtual void platformSetVisible(bool) = 0;
>     virtual void platformPlay() = 0;
>     virtual void platformPause() = 0;
>+    virtual bool platformPaused() const { return !rate(); }
>     virtual void checkPlayability() = 0;
>     virtual void seekToTime(const MediaTime&, const MediaTime& negativeTolerance, const MediaTime& positiveTolerance) = 0;
>     unsigned long long totalBytes() const override = 0;
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>index c7c93030c80e0376e3f8df857f211f7f4b18bed2..a2424dd80da2c43d767f3ade416cb7400bc54a57 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h
>@@ -167,6 +167,7 @@ private:
>     void platformSetVisible(bool) override;
>     void platformPlay() override;
>     void platformPause() override;
>+    bool platformPaused() const override;
>     MediaTime currentMediaTime() const override;
>     void setVolume(float) override;
> #if PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>index 2241db1522ac2c1b434a416031d23422e9c900ba..8ecafc53c3cbfc696b0212f454bad5ff03289975 100644
>--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
>@@ -590,6 +590,9 @@ void MediaPlayerPrivateAVFoundationObjC::cancelLoad()
>         setShouldObserveTimeControlStatus(false);
> 
>         [m_avPlayer replaceCurrentItemWithPlayerItem:nil];
>+#if !PLATFORM(IOS_FAMILY)
>+        [m_avPlayer setOutputContext:nil];
>+#endif
>         m_avPlayer = nil;
>     }
> 
>@@ -1304,6 +1307,11 @@ void MediaPlayerPrivateAVFoundationObjC::platformPause()
>     setPlayerRate(0);
> }
> 
>+bool MediaPlayerPrivateAVFoundationObjC::platformPaused() const
>+{
>+    return m_cachedTimeControlStatus == AVPlayerTimeControlStatusPaused;
>+}
>+
> MediaTime MediaPlayerPrivateAVFoundationObjC::platformDuration() const
> {
>     // Do not ask the asset for duration before it has been loaded or it will fetch the
>@@ -3267,8 +3275,10 @@ void MediaPlayerPrivateAVFoundationObjC::timeControlStatusDidChange(int timeCont
>         return;
> 
>     bool playerIsPlaying = m_cachedTimeControlStatus != AVPlayerTimeControlStatusPaused;
>-    if (playerIsPlaying != m_requestedPlaying)
>-        player()->handlePlaybackCommand(playerIsPlaying ? PlatformMediaSession::PlayCommand : PlatformMediaSession::PauseCommand);
>+    if (playerIsPlaying != m_requestedPlaying) {
>+        m_requestedPlaying = playerIsPlaying;
>+        player()->playbackStateChanged();
>+    }
> #endif
> }
> 
>@@ -3354,9 +3364,10 @@ void MediaPlayerPrivateAVFoundationObjC::setShouldObserveTimeControlStatus(bool
>         return;
> 
>     m_shouldObserveTimeControlStatus = shouldObserve;
>-    if (shouldObserve)
>+    if (shouldObserve) {
>         [m_avPlayer addObserver:m_objcObserver.get() forKeyPath:@"timeControlStatus" options:NSKeyValueObservingOptionNew context:(void *)MediaPlayerAVFoundationObservationContextPlayer];
>-    else
>+        timeControlStatusDidChange(m_avPlayer.get().timeControlStatus);
>+    } else
>         [m_avPlayer removeObserver:m_objcObserver.get() forKeyPath:@"timeControlStatus"];
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193499: 359281 | 359282 | 359308 | 359321 | 359324 | 359332 | 359333 | 359384