Update heartbeat behavior (#656)

Co-authored-by: Joaquim Stähli <[email protected]>
SRGSSR · Aug 12, 2024 · 066c1eb · 066c1eb
1 parent b3a566d
commit 066c1eb
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 287 deletions.
diff --git a/...in/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt b/...in/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt
@@ -15,6 +15,7 @@ import ch.srgssr.pillarbox.analytics.commandersact.TCMediaEvent
 import ch.srgssr.pillarbox.player.analytics.TotalPlaytimeCounter
 import ch.srgssr.pillarbox.player.extension.hasAccessibilityRoles
 import ch.srgssr.pillarbox.player.extension.isForced
+import ch.srgssr.pillarbox.player.runOnApplicationLooper
 import ch.srgssr.pillarbox.player.tracks.audioTracks
 import ch.srgssr.pillarbox.player.utils.DebugLogger
 import ch.srgssr.pillarbox.player.utils.Heartbeat
@@ -41,8 +42,10 @@ internal class CommandersActStreaming(
         period = POS_PERIOD,
         coroutineContext = coroutineContext,
         task = {
-            if (player.playWhenReady) {
-                notifyPos(player.currentPosition.milliseconds)
+            player.runOnApplicationLooper {
+                if (player.playWhenReady) {
+                    notifyPos(player.currentPosition.milliseconds)
+                }
             }
         },
     )
@@ -52,8 +55,10 @@ internal class CommandersActStreaming(
         period = UPTIME_PERIOD,
         coroutineContext = coroutineContext,
         task = {
-            if (player.playWhenReady && player.isCurrentMediaItemLive) {
-                notifyUptime(player.currentPosition.milliseconds)
+            player.runOnApplicationLooper {
+                if (player.playWhenReady && player.isCurrentMediaItemLive) {
+                    notifyUptime(player.currentPosition.milliseconds)
+                }
             }
         },
     )

diff --git a/...gssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt b/...gssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt
@@ -36,16 +36,14 @@ import io.mockk.mockk
 import io.mockk.slot
 import io.mockk.verify
 import io.mockk.verifyOrder
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestDispatcher
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
 import kotlinx.coroutines.test.advanceTimeBy
-import kotlinx.coroutines.test.resetMain
 import kotlinx.coroutines.test.runTest
-import kotlinx.coroutines.test.setMain
 import org.junit.runner.RunWith
 import org.robolectric.Shadows.shadowOf
+import kotlin.coroutines.EmptyCoroutineContext
 import kotlin.math.abs
 import kotlin.test.AfterTest
 import kotlin.test.BeforeTest
@@ -72,8 +70,6 @@ class CommandersActTrackerIntegrationTest {
         commandersAct = mockk(relaxed = true)
         testDispatcher = UnconfinedTestDispatcher()
 
-        Dispatchers.setMain(testDispatcher)
-
         val context = ApplicationProvider.getApplicationContext<Context>()
         val mediaItemTrackerRepository = DefaultMediaItemTrackerRepository(
             trackerRepository = MediaItemTrackerRepository(),
@@ -85,23 +81,21 @@ class CommandersActTrackerIntegrationTest {
         }
 
         val mediaCompositionWithFallbackService = LocalMediaCompositionWithFallbackService(context)
-
         player = DefaultPillarbox(
             context = context,
             mediaItemTrackerRepository = mediaItemTrackerRepository,
             mediaCompositionService = mediaCompositionWithFallbackService,
             clock = clock,
-            coroutineContext = testDispatcher,
+            // Use other CoroutineContext to avoid infinite loop because Heartbeat is also running in Pillarbox.
+            coroutineContext = EmptyCoroutineContext,
         )
     }
 
     @AfterTest
-    @OptIn(ExperimentalCoroutinesApi::class)
     fun tearDown() {
         clearAllMocks()
         player.release()
         shadowOf(Looper.getMainLooper()).idle()
-        Dispatchers.resetMain()
     }
 
     @Test

diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt
@@ -5,6 +5,7 @@
 package ch.srgssr.pillarbox.player
 
 import android.content.Context
+import android.os.Handler
 import androidx.annotation.VisibleForTesting
 import androidx.media3.common.C
 import androidx.media3.common.MediaItem
@@ -41,6 +42,8 @@ import ch.srgssr.pillarbox.player.tracker.MediaItemTrackerRepository
 import ch.srgssr.pillarbox.player.tracker.TimeRangeTracker
 import ch.srgssr.pillarbox.player.utils.PillarboxEventLogger
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.android.asCoroutineDispatcher
+import kotlinx.coroutines.runBlocking
 import kotlin.coroutines.CoroutineContext
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.milliseconds
@@ -136,7 +139,6 @@ class PillarboxExoPlayer internal constructor(
             sessionManager = sessionManager,
             coroutineContext = coroutineContext,
         )
-
         addListener(analyticsCollector)
         exoPlayer.addListener(ComponentListener())
         itemPillarboxDataTracker.addCallback(timeRangeTracker)
@@ -506,3 +508,18 @@ internal fun Window.isAtDefaultPosition(positionMs: Long): Boolean {
 private const val NormalSpeed = 1.0f
 
 private fun MediaItem.clearTag() = this.buildUpon().setTag(null).build()
+
+/**
+ * Run task in the same thread as [Player.getApplicationLooper] if it is needed.
+ *
+ * @param task The task to run.
+ */
+fun Player.runOnApplicationLooper(task: () -> Unit) {
+    if (applicationLooper.thread != Thread.currentThread()) {
+        runBlocking(Handler(applicationLooper).asCoroutineDispatcher("exoplayer")) {
+            task()
+        }
+    } else {
+        task()
+    }
+}
diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/PillarboxEventsDispatcher.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/PillarboxEventsDispatcher.kt
@@ -101,12 +101,6 @@ class PillarboxEventsDispatcher(
             DebugLogger.debug(TAG, "onPlayerReleased")
             notifyListeners { onPlayerReleased() }
         }
-
-        override fun onIsPlayingChanged(eventTime: EventTime, isPlaying: Boolean) {
-            val session = sessionManager.getSessionFromEventTime(eventTime) ?: return
-
-            notifyListeners { onIsPlaying(session, isPlaying) }
-        }
     }
 
     private companion object {

diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/QoSCoordinator.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/QoSCoordinator.kt
@@ -15,6 +15,7 @@ import ch.srgssr.pillarbox.player.analytics.PillarboxAnalyticsListener
 import ch.srgssr.pillarbox.player.analytics.PlaybackSessionManager
 import ch.srgssr.pillarbox.player.analytics.metrics.MetricsCollector
 import ch.srgssr.pillarbox.player.analytics.metrics.PlaybackMetrics
+import ch.srgssr.pillarbox.player.runOnApplicationLooper
 import ch.srgssr.pillarbox.player.utils.BitrateUtil.toByteRate
 import ch.srgssr.pillarbox.player.utils.DebugLogger
 import ch.srgssr.pillarbox.player.utils.Heartbeat
@@ -35,8 +36,9 @@ internal class QoSCoordinator(
         coroutineContext = coroutineContext,
         task = {
             val session = currentSession ?: return@Heartbeat
-
-            sendEvent("HEARTBEAT", session)
+            player.runOnApplicationLooper {
+                sendEvent("HEARTBEAT", session)
+            }
         },
     )
 
@@ -143,17 +145,6 @@ internal class QoSCoordinator(
         override fun onMediaStart(session: PlaybackSessionManager.Session) {
         }
 
-        override fun onIsPlaying(
-            session: PlaybackSessionManager.Session,
-            isPlaying: Boolean,
-        ) {
-            if (isPlaying) {
-                heartbeat.start(restart = false)
-            } else {
-                heartbeat.stop()
-            }
-        }
-
         override fun onSeek(session: PlaybackSessionManager.Session) {
             sendEvent("SEEK", session)
         }
@@ -204,7 +195,7 @@ internal class QoSCoordinator(
     }
 
     private companion object {
-        private val HEARTBEAT_PERIOD = 10.seconds
+        private val HEARTBEAT_PERIOD = 30.seconds
         private const val TAG = "QoSCoordinator"
     }
 }
diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcher.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcher.kt
@@ -22,17 +22,6 @@ interface QoSEventsDispatcher {
          */
         fun onMediaStart(session: PlaybackSessionManager.Session) = Unit
 
-        /**
-         * On is playing
-         *
-         * @param session
-         * @param isPlaying
-         */
-        fun onIsPlaying(
-            session: PlaybackSessionManager.Session,
-            isPlaying: Boolean,
-        ) = Unit
-
         /**
          * On seek
          *

diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/Heartbeat.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/utils/Heartbeat.kt
@@ -4,15 +4,12 @@
  */
 package ch.srgssr.pillarbox.player.utils
 
-import androidx.annotation.MainThread
 import kotlinx.coroutines.CoroutineName
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
 import kotlin.coroutines.CoroutineContext
 import kotlin.time.Duration
 
@@ -22,15 +19,15 @@ import kotlin.time.Duration
  * @param startDelay The initial delay before the first execution of [task].
  * @param period The period between two executions of [task].
  * @param coroutineContext The coroutine context in which [Heartbeat] is run.
- * @param task The task to execute, on the main [Thread] at regular [intervals][period].
+ * @param task The task to execute at regular [intervals][period].
  */
 class Heartbeat(
     private val startDelay: Duration = Duration.ZERO,
     private val period: Duration,
     private val coroutineContext: CoroutineContext,
-    @MainThread private val task: () -> Unit,
+    private val task: () -> Unit,
 ) {
-    private val coroutineScope = CoroutineScope(coroutineContext + CoroutineName("pillarbox-heart-beat"))
+    private val coroutineScope = CoroutineScope(coroutineContext + CoroutineName("pillarbox-heartbeat"))
 
     private var job: Job? = null
 
@@ -50,12 +47,8 @@ class Heartbeat(
 
         job = coroutineScope.launch {
             delay(startDelay)
-
             while (isActive) {
-                runBlocking(Dispatchers.Main) {
-                    task()
-                }
-
+                task()
                 delay(period)
             }
         }