Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix commanders act smooth seeking #456

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import ch.srgssr.pillarbox.player.extension.audio
import ch.srgssr.pillarbox.player.extension.isForced
import ch.srgssr.pillarbox.player.utils.DebugLogger
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import java.util.Timer
import kotlin.concurrent.fixedRateTimer
import kotlin.concurrent.scheduleAtFixedRate
Expand All @@ -36,7 +35,7 @@ internal class CommandersActStreaming(
) : AnalyticsListener {

private enum class State {
Idle, Playing, Paused, Seeking
Idle, Playing, Paused, HasSeek
}

private var state: State = State.Idle
Expand All @@ -57,13 +56,13 @@ internal class CommandersActStreaming(
name = "pillarbox-heart-beat", false, initialDelay = HEART_BEAT_DELAY.inWholeMilliseconds,
period = POS_PERIOD.inWholeMilliseconds
) {
MainScope().launch(Dispatchers.Main) {
runBlocking(Dispatchers.Main) {
notifyPos(player.currentPosition.milliseconds)
}
}.also {
if (!player.isCurrentMediaItemLive) return@also
it.scheduleAtFixedRate(HEART_BEAT_DELAY.inWholeMilliseconds, period = UPTIME_PERIOD.inWholeMilliseconds) {
MainScope().launch(Dispatchers.Main) {
runBlocking(Dispatchers.Main) {
notifyUptime(player.currentPosition.milliseconds)
}
}
Expand All @@ -85,7 +84,7 @@ internal class CommandersActStreaming(

override fun onEvents(player: Player, events: AnalyticsListener.Events) {
if (events.containsAny(AnalyticsListener.EVENT_PLAYBACK_STATE_CHANGED, AnalyticsListener.EVENT_PLAY_WHEN_READY_CHANGED)) {
if (player.playbackState != Player.STATE_READY) return
if (player.playbackState == Player.STATE_IDLE || player.playbackState == Player.STATE_ENDED) return
if (player.playWhenReady) {
notifyPlaying()
} else {
Expand All @@ -100,6 +99,7 @@ internal class CommandersActStreaming(
newPosition: Player.PositionInfo,
reason: Int
) {
if (!isPlaying()) return
when (reason) {
Player.DISCONTINUITY_REASON_SEEK, Player.DISCONTINUITY_REASON_SEEK_ADJUSTMENT -> {
if (abs(oldPosition.positionMs - newPosition.positionMs) > VALID_SEEK_THRESHOLD) {
Expand Down Expand Up @@ -156,7 +156,7 @@ internal class CommandersActStreaming(

private fun notifySeek(seekStartPosition: Duration) {
if (state != State.Playing) return
state = State.Seeking
state = State.HasSeek
notifyEvent(MediaEventType.Seek, seekStartPosition)
}

Expand All @@ -172,6 +172,10 @@ internal class CommandersActStreaming(
return if (position == ZERO) ZERO else player.duration.milliseconds - position
}

private fun isPlaying(): Boolean {
return player.playWhenReady && (player.playbackState == Player.STATE_READY || player.playbackState == Player.STATE_BUFFERING)
}

/**
* Handle text track data
* MEDIA_SUBTITLES_ON to true if text track selected but not forced, false otherwise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import io.mockk.verify
import io.mockk.verifyOrder
import org.junit.runner.RunWith
import kotlin.test.BeforeTest
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
Expand Down Expand Up @@ -101,7 +102,7 @@ class CommandersActTrackerIntegrationTest {
player.playWhenReady = true

TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)
TestPillarboxRunHelper.runUntilStartOfMediaItem(player, 0)
TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)

verifyOrder {
commandersAct.enableRunningInBackground()
Expand Down Expand Up @@ -169,6 +170,8 @@ class CommandersActTrackerIntegrationTest {

TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)

TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)

verifyOrder {
commandersAct.enableRunningInBackground()
}
Expand Down Expand Up @@ -388,18 +391,23 @@ class CommandersActTrackerIntegrationTest {
commandersAct.enableRunningInBackground()
commandersAct.sendTcMediaEvent(capture(tcMediaEvents))
commandersAct.sendTcMediaEvent(capture(tcMediaEvents))
commandersAct.sendTcMediaEvent(capture(tcMediaEvents))
}
confirmVerified(commandersAct)

assertEquals(2, tcMediaEvents.size)
assertEquals(3, tcMediaEvents.size)

assertEquals(MediaEventType.Seek, tcMediaEvents[0].eventType)
assertEquals(MediaEventType.Play, tcMediaEvents[0].eventType)
assertTrue(tcMediaEvents[0].assets.isNotEmpty())
assertNull(tcMediaEvents[0].sourceId)

assertEquals(MediaEventType.Play, tcMediaEvents[1].eventType)
assertEquals(MediaEventType.Seek, tcMediaEvents[1].eventType)
assertTrue(tcMediaEvents[1].assets.isNotEmpty())
assertNull(tcMediaEvents[1].sourceId)

assertEquals(MediaEventType.Play, tcMediaEvents[2].eventType)
assertTrue(tcMediaEvents[2].assets.isNotEmpty())
assertNull(tcMediaEvents[2].sourceId)
}

@Test
Expand Down Expand Up @@ -427,6 +435,7 @@ class CommandersActTrackerIntegrationTest {
verify { commandersAct wasNot Called }
}

@Ignore("Currently very flaky due to timer.")
@Test
fun `check uptime and position updates`() {
val delay = 2.seconds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ internal class CurrentMediaItemTracker internal constructor(
when (playbackState) {
Player.STATE_ENDED -> stopSession(MediaItemTracker.StopReason.EoF)
Player.STATE_IDLE -> stopSession(MediaItemTracker.StopReason.Stop)
Player.STATE_READY -> if (currentMediaItem == null) setMediaItem(player.currentMediaItem)
else -> {
// Nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,35 @@ class MediaItemTrackerTest {
confirmVerified(fakeMediaItemTracker)
}

@Test
fun `one MediaItem with mediaId and url set reach eof then seek back`() {
val mediaId = FakeMediaItemSource.MEDIA_ID_1
player.apply {
setMediaItem(
MediaItem.Builder()
.setMediaId(mediaId)
.setUri(FakeMediaItemSource.URL_MEDIA_1)
.build()
)
prepare()
play()
}

TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)
player.seekTo(FakeMediaItemSource.NEAR_END_POSITION_MS)
TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED)
player.seekBack()
TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)
TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)

verifyOrder {
fakeMediaItemTracker.start(any(), FakeMediaItemTracker.Data(mediaId))
fakeMediaItemTracker.stop(any(), MediaItemTracker.StopReason.EoF, player.duration)
fakeMediaItemTracker.start(any(), FakeMediaItemTracker.Data(mediaId))
}
confirmVerified(fakeMediaItemTracker)
}

@Test
fun `one MediaItem with mediaId and url set reach stop`() {
val mediaId = FakeMediaItemSource.MEDIA_ID_1
Expand Down
Loading