From 066c1eb176e5cd335b8a873c7810136a030c5dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Muller?= Date: Tue, 30 Jul 2024 10:00:42 +0200 Subject: [PATCH] Update heartbeat behavior (#656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Joaquim Stähli --- .../commandersact/CommandersActStreaming.kt | 13 +- .../CommandersActTrackerIntegrationTest.kt | 12 +- .../pillarbox/player/PillarboxExoPlayer.kt | 19 +- .../player/qos/PillarboxEventsDispatcher.kt | 6 - .../pillarbox/player/qos/QoSCoordinator.kt | 19 +- .../player/qos/QoSEventsDispatcher.kt | 11 - .../pillarbox/player/utils/Heartbeat.kt | 15 +- .../player/qos/QoSEventsDispatcherTest.kt | 204 ------------------ .../pillarbox/player/utils/HeartbeatTest.kt | 49 ++--- 9 files changed, 61 insertions(+), 287 deletions(-) delete mode 100644 pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcherTest.kt diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt index 4ab786c0e..42cf627b0 100644 --- a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt +++ b/pillarbox-core-business/src/main/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/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt index 1ce392351..e54e8e2da 100644 --- a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt +++ b/pillarbox-core-business/src/test/java/ch/srgssr/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() 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 index 5c8d28bbd..db83ac89d 100644 --- 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 index 24c48eac0..b55185ce8 100644 --- 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 index 533aed35f..ebbeb1ae1 100644 --- 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 index d1ddb0a3c..5181df49b 100644 --- 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 index 771736c00..a8bca4c15 100644 --- 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) } } diff --git a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcherTest.kt b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcherTest.kt deleted file mode 100644 index da1ce7788..000000000 --- a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/qos/QoSEventsDispatcherTest.kt +++ /dev/null @@ -1,204 +0,0 @@ -/* - * Copyright (c) SRG SSR. All rights reserved. - * License information is available from the LICENSE file. - */ -package ch.srgssr.pillarbox.player.qos - -import android.content.Context -import android.os.Looper -import androidx.media3.common.MediaItem -import androidx.media3.common.Player -import androidx.media3.exoplayer.ExoPlayer -import androidx.media3.test.utils.FakeClock -import androidx.media3.test.utils.robolectric.TestPlayerRunHelper -import androidx.test.core.app.ApplicationProvider -import androidx.test.ext.junit.runners.AndroidJUnit4 -import ch.srgssr.pillarbox.player.analytics.PlaybackSessionManager -import io.mockk.clearAllMocks -import io.mockk.clearMocks -import io.mockk.confirmVerified -import io.mockk.mockk -import io.mockk.verify -import org.junit.runner.RunWith -import org.robolectric.Shadows.shadowOf -import kotlin.test.AfterTest -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertEquals - -@RunWith(AndroidJUnit4::class) -class QoSEventsDispatcherTest { - private lateinit var clock: FakeClock - private lateinit var player: ExoPlayer - private lateinit var eventsDispatcherListener: QoSEventsDispatcher.Listener - - @BeforeTest - fun setUp() { - val context = ApplicationProvider.getApplicationContext() - - clock = FakeClock(true) - eventsDispatcherListener = mockk(relaxed = true) - player = ExoPlayer.Builder(context) - .setClock(clock) - .build() - .apply { - prepare() - } - - val sessionManager = PlaybackSessionManager().apply { - setPlayer(player) - } - - PillarboxEventsDispatcher(sessionManager).apply { - registerPlayer(player) - addListener(eventsDispatcherListener) - } - - clearMocks(eventsDispatcherListener) - } - - @AfterTest - fun tearDown() { - clearAllMocks() - player.release() - shadowOf(Looper.getMainLooper()).idle() - } - - @Test - fun `play single media item`() { - val mediaItem = MediaItem.fromUri(VOD1) - - player.setMediaItem(mediaItem) - player.play() - - TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) - - val onIsPlayingSessions = mutableListOf() - val onIsPlayingValue = mutableListOf() - - verify { - eventsDispatcherListener.onIsPlaying(capture(onIsPlayingSessions), capture(onIsPlayingValue)) - } - confirmVerified(eventsDispatcherListener) - - assertEquals(2, onIsPlayingValue.size) - assertEquals(1, onIsPlayingSessions.distinctBy { it.sessionId }.size) - assertEquals(listOf(true, false), onIsPlayingValue) - } - - @Test - fun `play multiple media items`() { - val mediaItems = listOf(VOD1, VOD2, VOD3).map { MediaItem.fromUri(it) } - - player.setMediaItems(mediaItems) - player.play() - - TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) - - // To ensure that the final `onSessionFinished` is triggered. - player.clearMediaItems() - - TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) - - val onIsPlayingSessions = mutableListOf() - val onIsPlayingValue = mutableListOf() - - verify { - eventsDispatcherListener.onIsPlaying(capture(onIsPlayingSessions), capture(onIsPlayingValue)) - } - confirmVerified(eventsDispatcherListener) - - assertEquals(6, onIsPlayingValue.size) - assertEquals(3, onIsPlayingSessions.distinctBy { it.sessionId }.size) - assertEquals(listOf(true, false, true, false, true, false), onIsPlayingValue) - } - - @Test - fun `play multiple media items, remove upcoming media item`() { - val mediaItems = listOf(VOD1, VOD2, VOD3).map { MediaItem.fromUri(it) } - - player.setMediaItems(mediaItems) - player.play() - player.removeMediaItem(player.currentMediaItemIndex + 1) - - TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) - - // To ensure that the final `onSessionFinished` is triggered. - player.clearMediaItems() - - TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) - - val onIsPlayingSessions = mutableListOf() - val onIsPlayingValue = mutableListOf() - - verify { - eventsDispatcherListener.onIsPlaying(capture(onIsPlayingSessions), capture(onIsPlayingValue)) - } - confirmVerified(eventsDispatcherListener) - - assertEquals(4, onIsPlayingValue.size) - assertEquals(2, onIsPlayingSessions.distinctBy { it.sessionId }.size) - assertEquals(listOf(true, false, true, false), onIsPlayingValue) - } - - @Test - fun `play multiple media items, remove current media item`() { - val mediaItems = listOf(VOD1, VOD2, VOD3).map { MediaItem.fromUri(it) } - - player.setMediaItems(mediaItems) - player.play() - player.removeMediaItem(player.currentMediaItemIndex) - - TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) - - // To ensure that the final `onSessionFinished` is triggered. - player.clearMediaItems() - - TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) - - val onIsPlayingSessions = mutableListOf() - val onIsPlayingValue = mutableListOf() - - verify { - eventsDispatcherListener.onIsPlaying(capture(onIsPlayingSessions), capture(onIsPlayingValue)) - } - confirmVerified(eventsDispatcherListener) - - assertEquals(4, onIsPlayingValue.size) - assertEquals(2, onIsPlayingSessions.distinctBy { it.sessionId }.size) - assertEquals(listOf(true, false, true, false), onIsPlayingValue) - } - - @Test - fun `play multiple same media items create multiple sessions`() { - val mediaItems = listOf(VOD1, VOD1, VOD3).map { MediaItem.fromUri(it) } - - player.setMediaItems(mediaItems) - player.play() - - TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) - - // To ensure that the final `onSessionFinished` is triggered. - player.clearMediaItems() - - TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) - - val onIsPlayingSessions = mutableListOf() - val onIsPlayingValue = mutableListOf() - - verify { - eventsDispatcherListener.onIsPlaying(capture(onIsPlayingSessions), capture(onIsPlayingValue)) - } - confirmVerified(eventsDispatcherListener) - - assertEquals(6, onIsPlayingValue.size) - assertEquals(3, onIsPlayingSessions.distinctBy { it.sessionId }.size) - assertEquals(listOf(true, false, true, false, true, false), onIsPlayingValue) - } - - private companion object { - private const val VOD1 = "https://rts-vod-amd.akamaized.net/ww/13444390/f1b478f7-2ae9-3166-94b9-c5d5fe9610df/master.m3u8" - private const val VOD2 = "https://rts-vod-amd.akamaized.net/ww/13444333/feb1d08d-e62c-31ff-bac9-64c0a7081612/master.m3u8" - private const val VOD3 = "https://rts-vod-amd.akamaized.net/ww/13444466/2787e520-412f-35fb-83d7-8dbb31b5c684/master.m3u8" - } -} diff --git a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/utils/HeartbeatTest.kt b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/utils/HeartbeatTest.kt index 7ed3d49e7..e47cc91bb 100644 --- a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/utils/HeartbeatTest.kt +++ b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/utils/HeartbeatTest.kt @@ -9,14 +9,11 @@ import android.os.Looper import androidx.media3.exoplayer.ExoPlayer import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -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.test.AfterTest @@ -41,16 +38,12 @@ class HeartbeatTest { @BeforeTest fun setUp() { - Dispatchers.setMain(testDispatcher) - taskRunsCount = 0 } @AfterTest fun tearDown() { shadowOf(Looper.getMainLooper()).idle() - - Dispatchers.resetMain() } @Test @@ -314,27 +307,29 @@ class HeartbeatTest { } @Test - fun `verify player is accessible from the task`() = runTest(testDispatcher) { + fun `verify player is accessible from the task`() { val context = ApplicationProvider.getApplicationContext() val player = ExoPlayer.Builder(context).build() - var taskCalled = false - - val heartbeat = Heartbeat( - period = 10.seconds, - coroutineContext = coroutineContext, - task = { - player.currentPosition - taskCalled = true - }, - ) - - heartbeat.start() - advanceTimeBy(15.seconds) - heartbeat.stop() - advanceTimeBy(15.seconds) - - assertTrue(taskCalled) - - player.release() + runTest(testDispatcher) { + var taskCalled = false + + val heartbeat = Heartbeat( + period = 10.seconds, + coroutineContext = coroutineContext, + task = { + player.currentPosition + taskCalled = true + }, + ) + + heartbeat.start() + advanceTimeBy(15.seconds) + heartbeat.stop() + advanceTimeBy(15.seconds) + + assertTrue(taskCalled) + + player.release() + } } }