Video sometimes flickers when playing to AppleTV
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Jul 2018 21:49:49 +0000 (21:49 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Jul 2018 21:49:49 +0000 (21:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187193
<rdar://problem/40153865>

Reviewed by Jer Noble and Youenn Fablet.
Source/WebCore:

No new tests, existing test updated.

Delay 100ms before changing the iOS audio session category because it is an expensive and
potentially disruptive operation, and changing an audio or video element configuration
can result in several quick, ultimately unnecessary, category changes.

* platform/audio/PlatformMediaSession.cpp:
(WebCore::PlatformMediaSession::clientWillBeginPlayback): Add logging.

* platform/audio/PlatformMediaSessionManager.cpp:
(WebCore::PlatformMediaSessionManager::PlatformMediaSessionManager): Initialize timer.
(WebCore::PlatformMediaSessionManager::removeSession): Deactivate audio session when there
are no sessions.
(WebCore::PlatformMediaSessionManager::updateSessionStateTimerFired): New, update session now.
(WebCore::PlatformMediaSessionManager::updateSessionState): Add parameter.
* platform/audio/PlatformMediaSessionManager.h:

* platform/audio/cocoa/MediaSessionManagerCocoa.cpp:
(PlatformMediaSessionManager::updateSessionState): Defer update if it isn't supposed to happen
immediately.

* platform/audio/ios/AudioSessionIOS.mm:
(WebCore::AudioSession::setCategory): Drive-by: setting the audio category to nil is a noop,
so don't waste time doing it.
(WebCore::AudioSession::tryToSetActive): Allow other apps to resume playback when we deactivate
the audio session.

* platform/Timer.h:
(WebCore::DeferrableOneShotTimer): Add WTF_MAKE_FAST_ALLOCATED so it can be used in a unique_ptr.

LayoutTests:

* platform/mac/media/audio-session-category-audio-autoplay.html: Update as audio category
doesn't change immediately.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/media/audio-session-category-audio-autoplay.html
Source/WebCore/ChangeLog
Source/WebCore/platform/Timer.h
Source/WebCore/platform/audio/PlatformMediaSession.cpp
Source/WebCore/platform/audio/PlatformMediaSessionManager.cpp
Source/WebCore/platform/audio/PlatformMediaSessionManager.h
Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.cpp
Source/WebCore/platform/audio/ios/AudioSessionIOS.mm

index 5968479957ffa5d235aab11268357a507e36a31f..f4f9af5c6a43133f4a1ebcc5497fb465bec6272b 100644 (file)
@@ -1,3 +1,14 @@
+2018-07-02  Eric Carlson  <eric.carlson@apple.com>
+
+        Video sometimes flickers when playing to AppleTV
+        https://bugs.webkit.org/show_bug.cgi?id=187193
+        <rdar://problem/40153865>
+
+        Reviewed by Jer Noble and Youenn Fablet.
+
+        * platform/mac/media/audio-session-category-audio-autoplay.html: Update as audio category
+        doesn't change immediately.
+
 2018-07-02  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed WPE gardening.
index 5d8f4bfc7130664766e29707cf6d07f11bbbb7cf..589a85856e1d3576bfb84b69c09433c00a6f7e1c 100644 (file)
@@ -13,8 +13,8 @@
         waitForEvent('playing', playing);
     }
 
-    function playing() {
-        testExpected('internals.audioSessionCategory()', 'MediaPlayback');
+    async function playing() {
+        await testExpectedEventually('internals.audioSessionCategory()', 'MediaPlayback');
         endTest();
     }
     </script>
index bc9f3ac105a446b59ab44393b8ded28619c2945f..dc3b94aa380669a3461f01e477f90d5f8b702cbc 100644 (file)
@@ -1,3 +1,41 @@
+2018-07-02  Eric Carlson  <eric.carlson@apple.com>
+
+        Video sometimes flickers when playing to AppleTV
+        https://bugs.webkit.org/show_bug.cgi?id=187193
+        <rdar://problem/40153865>
+
+        Reviewed by Jer Noble and Youenn Fablet.
+        
+        No new tests, existing test updated.
+
+        Delay 100ms before changing the iOS audio session category because it is an expensive and
+        potentially disruptive operation, and changing an audio or video element configuration
+        can result in several quick, ultimately unnecessary, category changes.
+
+        * platform/audio/PlatformMediaSession.cpp:
+        (WebCore::PlatformMediaSession::clientWillBeginPlayback): Add logging.
+
+        * platform/audio/PlatformMediaSessionManager.cpp:
+        (WebCore::PlatformMediaSessionManager::PlatformMediaSessionManager): Initialize timer.
+        (WebCore::PlatformMediaSessionManager::removeSession): Deactivate audio session when there
+        are no sessions.
+        (WebCore::PlatformMediaSessionManager::updateSessionStateTimerFired): New, update session now.
+        (WebCore::PlatformMediaSessionManager::updateSessionState): Add parameter.
+        * platform/audio/PlatformMediaSessionManager.h:
+
+        * platform/audio/cocoa/MediaSessionManagerCocoa.cpp:
+        (PlatformMediaSessionManager::updateSessionState): Defer update if it isn't supposed to happen
+        immediately.
+
+        * platform/audio/ios/AudioSessionIOS.mm:
+        (WebCore::AudioSession::setCategory): Drive-by: setting the audio category to nil is a noop,
+        so don't waste time doing it.
+        (WebCore::AudioSession::tryToSetActive): Allow other apps to resume playback when we deactivate
+        the audio session.
+
+        * platform/Timer.h:
+        (WebCore::DeferrableOneShotTimer): Add WTF_MAKE_FAST_ALLOCATED so it can be used in a unique_ptr.
+
 2018-07-02  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Clean up some spellchecking code
index 5e757d40555caf56cb0c0834ef3319013f29f24f..524d4861f2b48d30457bbf037fb5541499f50772 100644 (file)
@@ -146,6 +146,7 @@ inline bool TimerBase::isActive() const
 }
 
 class DeferrableOneShotTimer : protected TimerBase {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     template<typename TimerFiredClass>
     DeferrableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
index e319a7ae2b3cdd533df444f8667abe88c2bf3017..ee19b67530ecc1cbc742d5ce9435b71499a0eb88 100644 (file)
@@ -189,6 +189,8 @@ bool PlatformMediaSession::clientWillBeginPlayback()
     if (m_notifyingClient)
         return true;
 
+    INFO_LOG(LOGIDENTIFIER, "state = ", m_state);
+
     if (!PlatformMediaSessionManager::sharedManager().sessionWillBeginPlayback(*this)) {
         if (state() == Interrupted)
             m_stateToRestore = Playing;
index f06aa91d68bf351b449207af5bfe34991910f3db..ca72d42a974240db1e9f0190a0439b5a791a20c0 100644 (file)
@@ -116,7 +116,7 @@ void PlatformMediaSessionManager::beginInterruption(PlatformMediaSession::Interr
     forEachSession([type] (PlatformMediaSession& session, size_t) {
         session.beginInterruption(type);
     });
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 void PlatformMediaSessionManager::endInterruption(PlatformMediaSession::EndInterruptionFlags flags)
@@ -143,7 +143,7 @@ void PlatformMediaSessionManager::addSession(PlatformMediaSession& session)
     if (!m_audioHardwareListener)
         m_audioHardwareListener = AudioHardwareListener::create(*this);
 
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 void PlatformMediaSessionManager::removeSession(PlatformMediaSession& session)
@@ -162,9 +162,12 @@ void PlatformMediaSessionManager::removeSession(PlatformMediaSession& session)
     if (m_sessions.isEmpty() || std::all_of(m_sessions.begin(), m_sessions.end(), std::logical_not<void>())) {
         m_remoteCommandListener = nullptr;
         m_audioHardwareListener = nullptr;
+#if USE(AUDIO_SESSION)
+        AudioSession::sharedSession().tryToSetActive(false);
+#endif
     }
 
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 void PlatformMediaSessionManager::addRestriction(PlatformMediaSession::MediaType type, SessionRestrictions restriction)
@@ -250,7 +253,7 @@ void PlatformMediaSessionManager::sessionWillEndPlayback(PlatformMediaSession& s
 
 void PlatformMediaSessionManager::sessionStateChanged(PlatformMediaSession&)
 {
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 void PlatformMediaSessionManager::setCurrentSession(PlatformMediaSession& session)
@@ -290,7 +293,7 @@ Vector<PlatformMediaSession*> PlatformMediaSessionManager::currentSessionsMatchi
     });
     return matchingSessions;
 }
-    
+
 void PlatformMediaSessionManager::applicationWillBecomeInactive() const
 {
     LOG(Media, "PlatformMediaSessionManager::applicationWillBecomeInactive");
@@ -354,11 +357,11 @@ void PlatformMediaSessionManager::sessionIsPlayingToWirelessPlaybackTargetChange
 
 void PlatformMediaSessionManager::sessionCanProduceAudioChanged(PlatformMediaSession&)
 {
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 #if !PLATFORM(COCOA)
-void PlatformMediaSessionManager::updateSessionState()
+void PlatformMediaSessionManager::scheduleUpdateSessionState()
 {
 }
 #endif
@@ -401,7 +404,7 @@ void PlatformMediaSessionManager::systemDidWake()
 
 void PlatformMediaSessionManager::audioOutputDeviceChanged()
 {
-    updateSessionState();
+    scheduleUpdateSessionState();
 }
 
 void PlatformMediaSessionManager::stopAllMediaPlaybackForDocument(const Document* document)
index 8dcfd11106620fc9976685319fe83c84cfeb03fb..0e3dffd717e44d089ff75a880491945b9856de57 100644 (file)
@@ -29,6 +29,7 @@
 #include "AudioHardwareListener.h"
 #include "PlatformMediaSession.h"
 #include "RemoteCommandListener.h"
+#include "Timer.h"
 #include <pal/system/SystemSleepListener.h>
 #include <wtf/Vector.h>
 
@@ -125,7 +126,7 @@ protected:
 private:
     friend class Internals;
 
-    void updateSessionState();
+    void scheduleUpdateSessionState();
 
     // RemoteCommandListenerClient
     WEBCORE_EXPORT void didReceiveRemoteControlCommand(PlatformMediaSession::RemoteControlCommandType, const PlatformMediaSession::RemoteCommandArgument*) override;
@@ -151,6 +152,8 @@ private:
     bool m_canPlayToTarget { false };
 #endif
 
+    std::unique_ptr<DeferrableOneShotTimer> m_updateStateTimer;
+
     bool m_interrupted { false };
     mutable bool m_isApplicationInBackground { false };
     bool m_willIgnoreSystemInterruptions { false };
index 3fe59cf70ea5bb2cdd00f46c53a9a6cc7e0468f0..fbf2f29cf50872b6356c4af67ee4dfc1cb396c6c 100644 (file)
@@ -37,10 +37,11 @@ using namespace WebCore;
 
 static const size_t kWebAudioBufferSize = 128;
 static const size_t kLowPowerVideoBufferSize = 4096;
+static const Seconds updateSessionStateDelay { 100_ms };
 
-void PlatformMediaSessionManager::updateSessionState()
+void PlatformMediaSessionManager::scheduleUpdateSessionState()
 {
-    LOG(Media, "PlatformMediaSessionManager::updateSessionState() - types: Video(%d), Audio(%d), WebAudio(%d)", count(PlatformMediaSession::Video), count(PlatformMediaSession::Audio), count(PlatformMediaSession::WebAudio));
+    LOG(Media, "PlatformMediaSessionManager::scheduleUpdateSessionState() - types: Video(%d), Audio(%d), WebAudio(%d)", count(PlatformMediaSession::Video), count(PlatformMediaSession::Audio), count(PlatformMediaSession::WebAudio));
 
     if (has(PlatformMediaSession::WebAudio))
         AudioSession::sharedSession().setPreferredBufferSize(kWebAudioBufferSize);
@@ -64,25 +65,34 @@ void PlatformMediaSessionManager::updateSessionState()
     if (!DeprecatedGlobalSettings::shouldManageAudioSessionCategory())
         return;
 
-    bool hasWebAudioType = false;
-    bool hasAudibleAudioOrVideoMediaType = false;
-    bool hasAudioCapture = anyOfSessions([&hasWebAudioType, &hasAudibleAudioOrVideoMediaType] (PlatformMediaSession& session, size_t) mutable {
-        auto type = session.mediaType();
-        if (type == PlatformMediaSession::WebAudio)
-            hasWebAudioType = true;
-        if ((type == PlatformMediaSession::VideoAudio || type == PlatformMediaSession::Audio) && session.canProduceAudio() && session.state() == PlatformMediaSession::Playing)
-            hasAudibleAudioOrVideoMediaType = true;
-        return (type == PlatformMediaSession::MediaStreamCapturingAudio);
-    });
-
-    if (hasAudioCapture)
-        AudioSession::sharedSession().setCategory(AudioSession::PlayAndRecord);
-    else if (hasAudibleAudioOrVideoMediaType)
-        AudioSession::sharedSession().setCategory(AudioSession::MediaPlayback);
-    else if (hasWebAudioType)
-        AudioSession::sharedSession().setCategory(AudioSession::AmbientSound);
-    else
-        AudioSession::sharedSession().setCategory(AudioSession::None);
+    if (!m_updateStateTimer) {
+        auto updateSessionState = [this] () mutable {
+
+            bool hasWebAudioType = false;
+            bool hasAudibleAudioOrVideoMediaType = false;
+            bool hasAudioCapture = anyOfSessions([&hasWebAudioType, &hasAudibleAudioOrVideoMediaType] (PlatformMediaSession& session, size_t) mutable {
+                auto type = session.mediaType();
+                if (type == PlatformMediaSession::WebAudio)
+                    hasWebAudioType = true;
+                if ((type == PlatformMediaSession::VideoAudio || type == PlatformMediaSession::Audio) && session.canProduceAudio() && session.state() == PlatformMediaSession::Playing)
+                    hasAudibleAudioOrVideoMediaType = true;
+                return (type == PlatformMediaSession::MediaStreamCapturingAudio);
+            });
+
+            if (hasAudioCapture)
+                AudioSession::sharedSession().setCategory(AudioSession::PlayAndRecord);
+            else if (hasAudibleAudioOrVideoMediaType)
+                AudioSession::sharedSession().setCategory(AudioSession::MediaPlayback);
+            else if (hasWebAudioType)
+                AudioSession::sharedSession().setCategory(AudioSession::AmbientSound);
+            else
+                AudioSession::sharedSession().setCategory(AudioSession::None);
+        };
+
+        m_updateStateTimer = std::make_unique<DeferrableOneShotTimer>(WTFMove(updateSessionState), updateSessionStateDelay);
+    }
+
+    m_updateStateTimer->restart();
 }
 
 #endif // USE(AUDIO_SESSION)
index d5541df45b4177065213d98f2176f25370acb708..6c0ff533f1a02be34e86a8201ebff4ffce56ca9c 100644 (file)
@@ -135,9 +135,7 @@ void AudioSession::setCategory(CategoryType newCategory)
         categoryString = AVAudioSessionCategoryAudioProcessing;
         break;
     case None:
-    default:
-        categoryString = nil;
-        break;
+        return;
     }
 
     NSError *error = nil;
@@ -217,7 +215,7 @@ size_t AudioSession::numberOfOutputChannels() const
 bool AudioSession::tryToSetActive(bool active)
 {
     NSError *error = nil;
-    [[AVAudioSession sharedInstance] setActive:active error:&error];
+    [[AVAudioSession sharedInstance] setActive:active withOptions:active ? 0 : AVAudioSessionSetActiveOptionNotifyOthersOnDeactivation error:&error];
     return !error;
 }