From b40b156c41354839a5fe2fb1290cdf2c504b35dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaquim=20St=C3=A4hli?= Date: Tue, 12 Mar 2024 13:18:47 +0100 Subject: [PATCH] 464 fix analytics event skip next or previous media item (#465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gaëtan Muller --- .github/workflows/build.yml | 3 ++ .../CommandersActTrackerIntegrationTest.kt | 37 ++++++++++++++++- .../player/tracker/CurrentMediaItemTracker.kt | 29 +++++++------- .../player/tracker/MediaItemTrackerTest.kt | 40 ++++++++++++++++++- 4 files changed, 93 insertions(+), 16 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a10e75c61..ad9c913fa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,6 +16,9 @@ jobs: build: name: Build runs-on: ubuntu-latest + env: + USERNAME: ${{ github.actor }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v4 - name: Set up JDK 17 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 9bcbbf099..940d79607 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 @@ -168,7 +168,7 @@ class CommandersActTrackerIntegrationTest { } @Test - fun `audio URN don't send any analytics`() { + fun `audio URN send any analytics`() { val tcMediaEventSlot = slot() player.setMediaItem(MediaItemUrn(URN_AUDIO)) @@ -572,6 +572,41 @@ class CommandersActTrackerIntegrationTest { confirmVerified(commandersAct) } + @Test + fun `player seek to next item doesn't send seek event`() { + val tcMediaEvents = mutableListOf() + player.addMediaItem(MediaItemUrn(URN_NOT_LIVE_VIDEO)) + player.addMediaItem(MediaItemUrn(URN_VOD_SHORT)) + player.prepare() + player.play() + + TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY) + TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) + player.seekToNextMediaItem() + + TestPlayerRunHelper.runUntilTimelineChanged(player) + TestPlayerRunHelper.playUntilStartOfMediaItem(player, 1) + TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) + TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) + + verifyOrder { + commandersAct.enableRunningInBackground() + commandersAct.sendTcMediaEvent(capture(tcMediaEvents)) + commandersAct.sendTcMediaEvent(capture(tcMediaEvents)) + + commandersAct.enableRunningInBackground() + commandersAct.sendTcMediaEvent(capture(tcMediaEvents)) + commandersAct.sendTcMediaEvent(capture(tcMediaEvents)) + } + confirmVerified(commandersAct) + + assertEquals(4, tcMediaEvents.size) + + assertEquals(listOf(Play, Stop, Play, Eof).reversed(), tcMediaEvents.map { it.eventType }) + assertTrue(tcMediaEvents.all { it.assets.isNotEmpty() }) + assertTrue(tcMediaEvents.all { it.sourceId == null }) + } + @Test fun `player prepared and stopped`() { player.setMediaItem(MediaItemUrn(URN_LIVE_VIDEO)) diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemTracker.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemTracker.kt index 5259e2257..fab7a5c0d 100644 --- a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemTracker.kt +++ b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemTracker.kt @@ -7,8 +7,8 @@ package ch.srgssr.pillarbox.player.tracker import androidx.annotation.VisibleForTesting import androidx.media3.common.MediaItem import androidx.media3.common.Player -import androidx.media3.common.Timeline import androidx.media3.exoplayer.ExoPlayer +import androidx.media3.exoplayer.analytics.AnalyticsListener import ch.srgssr.pillarbox.player.extension.getMediaItemTrackerData import ch.srgssr.pillarbox.player.extension.getMediaItemTrackerDataOrNull @@ -28,7 +28,7 @@ import ch.srgssr.pillarbox.player.extension.getMediaItemTrackerDataOrNull internal class CurrentMediaItemTracker internal constructor( private val player: ExoPlayer, private val mediaItemTrackerProvider: MediaItemTrackerProvider -) : Player.Listener { +) : AnalyticsListener { /** * Trackers are null if tracking session is stopped! @@ -49,7 +49,7 @@ internal class CurrentMediaItemTracker internal constructor( } init { - player.addListener(this) + player.addAnalyticsListener(this) player.currentMediaItem?.let { startNewSession(it) } } @@ -126,11 +126,11 @@ internal class CurrentMediaItemTracker internal constructor( } } - override fun onTimelineChanged(timeline: Timeline, reason: Int) { + override fun onTimelineChanged(eventTime: AnalyticsListener.EventTime, reason: Int) { setMediaItem(player.currentMediaItem) } - override fun onPlaybackStateChanged(playbackState: Int) { + override fun onPlaybackStateChanged(eventTime: AnalyticsListener.EventTime, playbackState: Int) { when (playbackState) { Player.STATE_ENDED -> stopSession(MediaItemTracker.StopReason.EoF) Player.STATE_IDLE -> stopSession(MediaItemTracker.StopReason.Stop) @@ -141,24 +141,25 @@ internal class CurrentMediaItemTracker internal constructor( } } - override fun onPositionDiscontinuity(oldPosition: Player.PositionInfo, newPosition: Player.PositionInfo, reason: Int) { + override fun onPositionDiscontinuity( + eventTime: AnalyticsListener.EventTime, + oldPosition: Player.PositionInfo, + newPosition: Player.PositionInfo, + reason: Int + ) { val oldPositionMs = oldPosition.positionMs when (reason) { Player.DISCONTINUITY_REASON_REMOVE -> stopSession(MediaItemTracker.StopReason.Stop, oldPositionMs) Player.DISCONTINUITY_REASON_AUTO_TRANSITION -> stopSession(MediaItemTracker.StopReason.EoF, oldPositionMs) else -> { - // Nothing + if (oldPosition.mediaItemIndex != newPosition.mediaItemIndex) { + stopSession(MediaItemTracker.StopReason.Stop, oldPositionMs) + } } } } - /** - * On media item transition - * - * @param mediaItem maybe null when playlist become empty - * @param reason - */ - override fun onMediaItemTransition(mediaItem: MediaItem?, reason: Int) { + override fun onMediaItemTransition(eventTime: AnalyticsListener.EventTime, mediaItem: MediaItem?, reason: Int) { setMediaItem(player.currentMediaItem) } diff --git a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/tracker/MediaItemTrackerTest.kt b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/tracker/MediaItemTrackerTest.kt index d10681481..cf5550d93 100644 --- a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/tracker/MediaItemTrackerTest.kt +++ b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/tracker/MediaItemTrackerTest.kt @@ -611,6 +611,45 @@ class MediaItemTrackerTest { confirmVerified(fakeMediaItemTracker) } + @Test + fun `playlist skip next stop current tracker`() { + val firstMediaId = FakeMediaItemSource.MEDIA_ID_1 + val secondMediaId = FakeMediaItemSource.MEDIA_ID_2 + player.apply { + addMediaItem( + MediaItem.Builder() + .setMediaId(firstMediaId) + .build() + ) + addMediaItem( + MediaItem.Builder() + .setMediaId(secondMediaId) + .build() + ) + prepare() + play() + } + TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY) + + RobolectricUtil.runMainLooperUntil { + val item = player.currentMediaItem + item?.getMediaItemTrackerDataOrNull() != null + } + + player.seekToNextMediaItem() + + TestPlayerRunHelper.runUntilTimelineChanged(player) + + TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player) + + verifyOrder { + fakeMediaItemTracker.start(player, FakeMediaItemTracker.Data(firstMediaId)) + fakeMediaItemTracker.stop(player, MediaItemTracker.StopReason.Stop, any()) + fakeMediaItemTracker.start(player, FakeMediaItemTracker.Data(secondMediaId)) + } + confirmVerified(fakeMediaItemTracker) + } + @Test fun `playlist repeat current item reset current tracker`() { val firstMediaId = FakeMediaItemSource.MEDIA_ID_1 @@ -637,7 +676,6 @@ class MediaItemTrackerTest { fakeMediaItemTracker.start(any(), FakeMediaItemTracker.Data(firstMediaId)) fakeMediaItemTracker.stop(any(), MediaItemTracker.StopReason.EoF, any()) fakeMediaItemTracker.start(any(), FakeMediaItemTracker.Data(firstMediaId)) - // player.stop fakeMediaItemTracker.stop(any(), MediaItemTracker.StopReason.Stop, any()) } confirmVerified(fakeMediaItemTracker)