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

Add setMediaListAndPlay to PlayerRepository #497

Merged
merged 1 commit into from
Aug 16, 2022
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 @@ -19,6 +19,7 @@ package com.google.android.horologist.media.data.repository
import android.util.Log
import androidx.media3.common.MediaItem
import androidx.media3.common.Player
import androidx.media3.common.Timeline
import com.google.android.horologist.media.data.ExperimentalHorologistMediaDataApi
import com.google.android.horologist.media.data.mapper.MediaItemMapper
import com.google.android.horologist.media.data.mapper.MediaMapper
Expand Down Expand Up @@ -90,13 +91,17 @@ public class PlayerRepositoryImpl(

private var _playbackSpeed = MutableStateFlow(1f)

// https://github.com/google/horologist/issues/496
private var mediaIndexToSeekTo: Int? = null

/**
* The current playback speed relative to 1.0.
*/
public val playbackSpeed: StateFlow<Float>
get() = _playbackSpeed

private val listener = object : Player.Listener {

override fun onEvents(player: Player, events: Player.Events) {
if (events.contains(Player.EVENT_AVAILABLE_COMMANDS_CHANGED)) {
updateAvailableCommands(player)
Expand Down Expand Up @@ -125,6 +130,19 @@ public class PlayerRepositoryImpl(
updateState(player)
}
}

// https://github.com/google/horologist/issues/496
override fun onTimelineChanged(timeline: Timeline, reason: Int) {
mediaIndexToSeekTo?.let { index ->
_player.value?.let { player ->
player.seekTo(index, 0)
player.prepare()
player.play()
}

mediaIndexToSeekTo = null
}
}
}

private fun updatePlaybackSpeed(player: Player) {
Expand Down Expand Up @@ -315,6 +333,16 @@ public class PlayerRepositoryImpl(
}
}

override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
checkNotClosed()

player.value?.let {
it.setMediaItems(mediaList.map(mediaItemMapper::map))

mediaIndexToSeekTo = index
}
}

override fun addMedia(media: Media) {
checkNotClosed()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ class PlayerRepositoryImplCloseTest(
param("setMediaList") { sut: PlayerRepositoryImpl, _: Context ->
sut.setMediaList(listOf(getDummyMedia()))
},
param("setMediaListAndPlay") { sut: PlayerRepositoryImpl, _: Context ->
sut.setMediaListAndPlay(listOf(getDummyMedia(), getDummyMedia()), 1)
},
param("addMedia") { sut: PlayerRepositoryImpl, _: Context ->
sut.addMedia(getDummyMedia())
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ class PlayerRepositoryImplNotConnectedTest(
param("setMedia") { sut: PlayerRepositoryImpl ->
sut.setMedia(getDummyMedia())
},
param("setMediaListAndPlay") { sut: PlayerRepositoryImpl ->
sut.setMediaListAndPlay(listOf(getDummyMedia(), getDummyMedia()), 1)
},
param("setMediaList") { sut: PlayerRepositoryImpl ->
sut.setMediaList(listOf(getDummyMedia()))
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import com.google.common.truth.Truth.assertThat
import org.junit.Assert.assertThrows
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Ignore
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
Expand Down Expand Up @@ -564,6 +565,36 @@ class PlayerRepositoryImplTest {
)
}

@Ignore("Fix test once this issue is fixed: https://github.com/androidx/media/issues/85")
@Test
fun `given NO previous MediaList is set when setMediaListAndPlay then state is correct`() {
// given
val player = TestExoPlayerBuilder(context).build()
sut.connect(player) {}

val media1 = getStubMedia("id1")
val media2 = getStubMedia("id2")
val mediaList = listOf(media1, media2)

// when
sut.setMediaListAndPlay(mediaList, 1)
runUntilPendingCommandsAreFullyHandled(player)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know we can rely on this still? Does runUntilPendingCommandsAreFullyHandled, wait until the items are resolved and then we get onTimelineChangedcalled on the main thread?


// then
assertThat(sut.getMediaCount()).isEqualTo(2)
assertThat(sut.getMediaAt(0)).isEqualTo(media1)
assertThat(sut.getMediaAt(1)).isEqualTo(media2)
assertThat(sut.currentState.value).isEqualTo(PlayerState.Playing)
assertThat(sut.currentMedia.value).isEqualTo(media2)
assertThat(sut.playbackSpeed.value).isEqualTo(1f)
assertThat(sut.shuffleModeEnabled.value).isFalse()
assertThat(sut.player.value).isSameInstanceAs(player)
assertThat(sut.mediaPosition.value).isEqualTo(MediaPosition.UnknownDuration(0.seconds))
assertThat(sut.availableCommands.value).containsExactlyElementsIn(
listOf(Command.PlayPause, Command.SkipToPreviousMedia, Command.SetShuffle)
)
}

@Test
fun `given NO previous Media is set when addMedia then state is correct`() {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ class FakePlayerRepository : PlayerRepository {
_currentMedia.value = mediaList[currentItemIndex]
}

override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
// do nothing
}

override fun addMedia(media: Media) {
// do nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import kotlin.time.Duration.Companion.seconds

@OptIn(ExperimentalHorologistMediaApi::class)
class StubPlayerRepository : PlayerRepository {

override val connected: StateFlow<Boolean>
get() = MutableStateFlow(false)

Expand Down Expand Up @@ -99,6 +100,10 @@ class StubPlayerRepository : PlayerRepository {
// do nothing
}

override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
// do nothing
}

override fun addMedia(media: Media) {
// do nothing
}
Expand Down
1 change: 1 addition & 0 deletions media/api/current.api
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ package com.google.android.horologist.media.repository {
method public void seekToDefaultPosition(int mediaIndex);
method public void setMedia(com.google.android.horologist.media.model.Media media);
method public void setMediaList(java.util.List<com.google.android.horologist.media.model.Media> mediaList);
method public void setMediaListAndPlay(java.util.List<com.google.android.horologist.media.model.Media> mediaList, int index);
method public void setShuffleModeEnabled(boolean shuffleModeEnabled);
method public void skipToNextMedia();
method public void skipToPreviousMedia();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,19 @@ public interface PlayerRepository {
* Clears the playlist, adds the specified [Media] list and resets the position to
* the default position.
*
* @param mediaList The new [Media].
* @param mediaList The new [Media] list.
*/
public fun setMediaList(mediaList: List<Media>)

/**
* Clears the playlist, adds the specified [Media] list and plays the [Media] at the position of
* the index passed as param.
*
* @param mediaList The new [Media] list.
* @param index The new position.
*/
public fun setMediaListAndPlay(mediaList: List<Media>, index: Int)

/**
* Adds a [Media] to the end of the playlist.
*/
Expand Down