From aee9477336375b8f6ae11f451724e1d37b4a3dd8 Mon Sep 17 00:00:00 2001 From: Jorge Antonio Diaz-Benito Soriano Date: Mon, 24 Jun 2024 17:43:30 +0200 Subject: [PATCH] Fix duplicated action issue found in test --- .../playbackengine/ExoPlayerPlaybackEngine.kt | 16 ++++++--- .../streamingsession/PlaybackSession.kt | 29 +++++++-------- .../ExoPlayerPlaybackEngineTest.kt | 35 ++++++------------- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt index c9cdb020..59736f02 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt @@ -460,7 +460,7 @@ internal class ExoPlayerPlaybackEngine( } else { Action.Type.PLAYBACK_STOP } - currentPlaybackSession?.actions?.add( + currentPlaybackSession?.tryAddAction( Action( trueTimeWrapper.currentTimeMillis, positionInSeconds, @@ -594,9 +594,15 @@ internal class ExoPlayerPlaybackEngine( }.toDouble() / MS_IN_SECOND startStall(Stall.Reason.SEEK, stallPositionSeconds, invokedAtMillis) } - currentPlaybackSession?.actions?.apply { - add(Action(invokedAtMillis, oldPositionSeconds, Action.Type.PLAYBACK_STOP)) - add(Action(invokedAtMillis, newPositionSeconds, Action.Type.PLAYBACK_START)) + currentPlaybackSession?.apply { + tryAddAction(Action(invokedAtMillis, oldPositionSeconds, Action.Type.PLAYBACK_STOP)) + tryAddAction( + Action( + invokedAtMillis, + newPositionSeconds, + Action.Type.PLAYBACK_START, + ), + ) } } } @@ -674,7 +680,7 @@ internal class ExoPlayerPlaybackEngine( } else { eventTime.currentPlaybackPositionMs }.toDouble() / MS_IN_SECOND - currentPlaybackSession?.actions?.add( + currentPlaybackSession?.tryAddAction( Action( trueTimeWrapper.currentTimeMillis, positionInSeconds, diff --git a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/mediasource/streamingsession/PlaybackSession.kt b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/mediasource/streamingsession/PlaybackSession.kt index 0e78bb06..03b27ac1 100644 --- a/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/mediasource/streamingsession/PlaybackSession.kt +++ b/player/playback-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/mediasource/streamingsession/PlaybackSession.kt @@ -17,7 +17,7 @@ internal sealed class PlaybackSession { abstract val actualQuality: ProductQuality abstract val sourceType: String? abstract val sourceId: String? - abstract val actions: MutableList + val actions: List = mutableListOf() var startTimestamp = 0L var startAssetPosition by @@ -25,6 +25,17 @@ internal sealed class PlaybackSession { oldValue == START_ASSET_POSITION_UNASSIGNED && newValue >= 0 } + fun tryAddAction(action: Action) { + if (actions.lastOrNull()?.actionType == action.actionType) { + /** + * PlayLog-wise, we can't start while started or stop while stopped. However, + * ExoPlayer-wise, it can happen for example when a discontinuity occurs while paused. + */ + return + } + (actions as MutableList).add(action) + } + @Suppress("LongParameterList") class Audio( override val playbackSessionId: UUID, @@ -35,10 +46,7 @@ internal sealed class PlaybackSession { override val actualQuality: AudioQuality, override val sourceType: String?, override val sourceId: String?, - ) : PlaybackSession() { - - override val actions = mutableListOf() - } + ) : PlaybackSession() @Suppress("LongParameterList") class Video( @@ -49,10 +57,7 @@ internal sealed class PlaybackSession { override val actualQuality: VideoQuality, override val sourceType: String?, override val sourceId: String?, - ) : PlaybackSession() { - - override val actions = mutableListOf() - } + ) : PlaybackSession() class Broadcast( override val playbackSessionId: UUID, @@ -61,10 +66,7 @@ internal sealed class PlaybackSession { override val actualQuality: AudioQuality, override val sourceType: String?, override val sourceId: String?, - ) : PlaybackSession() { - - override val actions = mutableListOf() - } + ) : PlaybackSession() @Suppress("LongParameterList") class UC( @@ -76,7 +78,6 @@ internal sealed class PlaybackSession { ) : PlaybackSession() { override val actualQuality = AudioQuality.LOW - override val actions = mutableListOf() } companion object { diff --git a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt index 3ba1ac7a..c8dd9911 100644 --- a/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt +++ b/player/playback-engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt @@ -899,10 +899,7 @@ internal class ExoPlayerPlaybackEngineTest { } else { currentPlaybackPositionMs }.toDouble() / 1_000 - val actions = mock>() - val currentPlaybackSession = mock { - on { it.actions } doReturn actions - } + val currentPlaybackSession = mock() playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession val currentTimeMills = -1L whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills @@ -925,9 +922,8 @@ internal class ExoPlayerPlaybackEngineTest { } verify(initialExtendedExoPlayer).currentPositionSinceEpochMs } - verify(currentPlaybackSession).actions verify(trueTimeWrapper).currentTimeMillis - verify(actions).add( + verify(currentPlaybackSession).tryAddAction( Action( currentTimeMills, positionSeconds, @@ -939,7 +935,6 @@ internal class ExoPlayerPlaybackEngineTest { mediaSource, eventTime, currentPlaybackSession, - actions, initialExtendedExoPlayer, ) } @@ -1719,10 +1714,7 @@ internal class ExoPlayerPlaybackEngineTest { whenever(extendedExoPlayer.shouldStartPlaybackAfterUserAction()) doReturn false val currentTimeMills = -80L whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills - val actions = mock>() - val currentPlaybackSession = mock { - on { it.actions } doReturn actions - } + val currentPlaybackSession = mock() playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession playbackEngine.onPositionDiscontinuity( @@ -1735,16 +1727,15 @@ internal class ExoPlayerPlaybackEngineTest { verify(extendedExoPlayer).updatePosition(newPositionMs) verify(extendedExoPlayer).shouldStartPlaybackAfterUserAction() verify(trueTimeWrapper).currentTimeMillis - verify(currentPlaybackSession).actions - inOrder(actions).apply { - verify(actions).add( + inOrder(currentPlaybackSession).apply { + verify(currentPlaybackSession).tryAddAction( Action( currentTimeMills, oldPositionMs.toDouble() / 1_000, Action.Type.PLAYBACK_STOP, ), ) - verify(actions).add( + verify(currentPlaybackSession).tryAddAction( Action( currentTimeMills, newPositionMs.toDouble() / 1_000, @@ -1763,7 +1754,6 @@ internal class ExoPlayerPlaybackEngineTest { oldPosition, newPosition, extendedExoPlayer, - actions, currentPlaybackSession, ) } @@ -1803,10 +1793,7 @@ internal class ExoPlayerPlaybackEngineTest { whenever(extendedExoPlayer.shouldStartPlaybackAfterUserAction()) doReturn true val currentTimeMills = -80L whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills - val actions = mock>() - val currentPlaybackSession = mock { - on { it.actions } doReturn actions - } + val currentPlaybackSession = mock() playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession playbackEngine.onPositionDiscontinuity( @@ -1819,16 +1806,15 @@ internal class ExoPlayerPlaybackEngineTest { verify(extendedExoPlayer).updatePosition(newPositionMs) verify(extendedExoPlayer).shouldStartPlaybackAfterUserAction() verify(trueTimeWrapper).currentTimeMillis - verify(currentPlaybackSession).actions - inOrder(actions).apply { - verify(actions).add( + inOrder(currentPlaybackSession).apply { + verify(currentPlaybackSession).tryAddAction( Action( currentTimeMills, oldPositionMs.toDouble() / 1_000, Action.Type.PLAYBACK_STOP, ), ) - verify(actions).add( + verify(currentPlaybackSession).tryAddAction( Action( currentTimeMills, newPositionMs.toDouble() / 1_000, @@ -1842,7 +1828,6 @@ internal class ExoPlayerPlaybackEngineTest { oldPosition, newPosition, extendedExoPlayer, - actions, currentPlaybackSession, ) }