Skip to content

Commit

Permalink
Fix duplicated action issue found in test
Browse files Browse the repository at this point in the history
  • Loading branch information
@stoyicker
stoyicker committed Jun 27, 2024
1 parent e5575f9 commit a9723fb
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 44 deletions.
16 changes: 11 additions & 5 deletions ...ack-engine/src/main/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngine.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ internal class ExoPlayerPlaybackEngine(
} else {
Action.Type.PLAYBACK_STOP
}
currentPlaybackSession?.actions?.add(
currentPlaybackSession?.tryAddAction(
Action(
trueTimeWrapper.currentTimeMillis,
positionInSeconds,
Expand Down Expand Up @@ -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,
),
)
}
}
}
Expand Down Expand Up @@ -674,7 +680,7 @@ internal class ExoPlayerPlaybackEngine(
} else {
eventTime.currentPlaybackPositionMs
}.toDouble() / MS_IN_SECOND
currentPlaybackSession?.actions?.add(
currentPlaybackSession?.tryAddAction(
Action(
trueTimeWrapper.currentTimeMillis,
positionInSeconds,
Expand Down
29 changes: 15 additions & 14 deletions ...otlin/com/tidal/sdk/player/playbackengine/mediasource/streamingsession/PlaybackSession.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,25 @@ internal sealed class PlaybackSession {
abstract val actualQuality: ProductQuality
abstract val sourceType: String?
abstract val sourceId: String?
abstract val actions: MutableList<Action>
val actions: List<Action> = mutableListOf()

var startTimestamp = 0L
var startAssetPosition by
Delegates.vetoable(START_ASSET_POSITION_UNASSIGNED) { _, oldValue, newValue ->
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<Action>).add(action)
}

@Suppress("LongParameterList")
class Audio(
override val playbackSessionId: UUID,
Expand All @@ -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<Action>()
}
) : PlaybackSession()

@Suppress("LongParameterList")
class Video(
Expand All @@ -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<Action>()
}
) : PlaybackSession()

class Broadcast(
override val playbackSessionId: UUID,
Expand All @@ -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<Action>()
}
) : PlaybackSession()

@Suppress("LongParameterList")
class UC(
Expand All @@ -76,7 +78,6 @@ internal sealed class PlaybackSession {
) : PlaybackSession() {

override val actualQuality = AudioQuality.LOW
override val actions = mutableListOf<Action>()
}

companion object {
Expand Down
35 changes: 10 additions & 25 deletions ...engine/src/test/kotlin/com/tidal/sdk/player/playbackengine/ExoPlayerPlaybackEngineTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -899,10 +899,7 @@ internal class ExoPlayerPlaybackEngineTest {
} else {
currentPlaybackPositionMs
}.toDouble() / 1_000
val actions = mock<MutableList<Action>>()
val currentPlaybackSession = mock<PlaybackSession.Audio> {
on { it.actions } doReturn actions
}
val currentPlaybackSession = mock<PlaybackSession.Audio>()
playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession
val currentTimeMills = -1L
whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills
Expand All @@ -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,
Expand All @@ -939,7 +935,6 @@ internal class ExoPlayerPlaybackEngineTest {
mediaSource,
eventTime,
currentPlaybackSession,
actions,
initialExtendedExoPlayer,
)
}
Expand Down Expand Up @@ -1719,10 +1714,7 @@ internal class ExoPlayerPlaybackEngineTest {
whenever(extendedExoPlayer.shouldStartPlaybackAfterUserAction()) doReturn false
val currentTimeMills = -80L
whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills
val actions = mock<MutableList<Action>>()
val currentPlaybackSession = mock<PlaybackSession.Audio> {
on { it.actions } doReturn actions
}
val currentPlaybackSession = mock<PlaybackSession.Audio>()
playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession

playbackEngine.onPositionDiscontinuity(
Expand All @@ -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,
Expand All @@ -1763,7 +1754,6 @@ internal class ExoPlayerPlaybackEngineTest {
oldPosition,
newPosition,
extendedExoPlayer,
actions,
currentPlaybackSession,
)
}
Expand Down Expand Up @@ -1803,10 +1793,7 @@ internal class ExoPlayerPlaybackEngineTest {
whenever(extendedExoPlayer.shouldStartPlaybackAfterUserAction()) doReturn true
val currentTimeMills = -80L
whenever(trueTimeWrapper.currentTimeMillis) doReturn currentTimeMills
val actions = mock<MutableList<Action>>()
val currentPlaybackSession = mock<PlaybackSession.Audio> {
on { it.actions } doReturn actions
}
val currentPlaybackSession = mock<PlaybackSession.Audio>()
playbackEngine.reflectionCurrentPlaybackSession = currentPlaybackSession

playbackEngine.onPositionDiscontinuity(
Expand All @@ -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,
Expand All @@ -1842,7 +1828,6 @@ internal class ExoPlayerPlaybackEngineTest {
oldPosition,
newPosition,
extendedExoPlayer,
actions,
currentPlaybackSession,
)
}
Expand Down

0 comments on commit a9723fb

Please sign in to comment.