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 5968479..f4f9af5 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 5d8f4bf..589a858 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 bc9f3ac..dc3b94a 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 5e757d4..524d486 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 e319a7a..ee19b67 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 f06aa91..ca72d42 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 8dcfd11..0e3dffd 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 3fe59cf..fbf2f29 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 d5541df..6c0ff53 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;
 }