diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 779ba7e21d..4a91a46a51 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -43,6 +43,8 @@ ([#99](https://github.com/androidx/media/issues/99)). * `SimpleBitmapLoader` can load bitmap from `file://` URIs ([#108](https://github.com/androidx/media/issues/108)). + * Fix assertion that prevents `MediaController` to seek over an ad in a + period ([#122](https://github.com/androidx/media/issues/122)). * Fix bug where the `MediaSessionService` was kept started in the foreground and a notification with a pause button was still shown when the player completed playback. Now, once playback is completed, the diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index d217afa06e..1484843bc0 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -2674,7 +2674,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; return playerInfo; } - checkState(!isAd(newPeriod, newPositionUs)); + checkState(playerInfo.sessionPositionInfo.positionInfo.adGroupIndex == C.INDEX_UNSET); PositionInfo oldPositionInfo = new PositionInfo( @@ -2685,8 +2685,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; oldPeriodIndex, /* positionMs= */ usToMs(oldPeriod.positionInWindowUs + oldPositionUs), /* contentPositionMs= */ usToMs(oldPeriod.positionInWindowUs + oldPositionUs), - playerInfo.sessionPositionInfo.positionInfo.adGroupIndex, - playerInfo.sessionPositionInfo.positionInfo.adIndexInAdGroup); + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET); timeline.getPeriod(newPeriodIndex, newPeriod); Window newWindow = new Window(); @@ -2700,8 +2700,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; newPeriodIndex, /* positionMs= */ usToMs(newPeriod.positionInWindowUs + newPositionUs), /* contentPositionMs= */ usToMs(newPeriod.positionInWindowUs + newPositionUs), - playerInfo.sessionPositionInfo.positionInfo.adGroupIndex, - playerInfo.sessionPositionInfo.positionInfo.adIndexInAdGroup); + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET); playerInfo = playerInfo.copyWithPositionInfos( oldPositionInfo, newPositionInfo, DISCONTINUITY_REASON_SEEK); @@ -2768,10 +2768,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; return getPeriodInfo(timeline, window, period, windowIndex, Util.msToUs(windowPositionMs)); } - private boolean isAd(Period period, long periodPosition) { - return period.getAdGroupIndexForPositionUs(periodPosition) != C.INDEX_UNSET; - } - @Nullable private PeriodInfo getPeriodInfo( Timeline timeline, Window window, Period period, int windowIndex, long windowPositionUs) { diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java index ad7687d4af..19cac9faa0 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java @@ -404,7 +404,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; !currentTimeline.isEmpty() ? currentTimeline.getWindow(newMediaItemIndex, new Window()).mediaItem : null, - newPositionMs); + newPositionMs, + /* isPlayingAd= */ false); PlayerInfo maskedPlayerInfo = controllerInfo.playerInfo.copyWithSessionPositionInfo( createSessionPositionInfo( @@ -656,7 +657,11 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; controllerInfo.playerInfo.copyWithTimelineAndSessionPositionInfo( newQueueTimeline, createSessionPositionInfo( - createPositionInfo(startIndex, mediaItems.get(startIndex), startPositionMs), + createPositionInfo( + startIndex, + mediaItems.get(startIndex), + startPositionMs, + /* isPlayingAd= */ false), /* isPlayingAd= */ false, /* durationMs= */ C.TIME_UNSET, /* bufferedPositionMs= */ 0, @@ -2089,7 +2094,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; @Nullable MediaItem currentMediaItem = currentTimeline.getMediaItemAt(currentMediaItemIndex); PositionInfo positionInfo = - createPositionInfo(currentMediaItemIndex, currentMediaItem, currentPositionMs); + createPositionInfo(currentMediaItemIndex, currentMediaItem, currentPositionMs, isPlayingAd); SessionPositionInfo sessionPositionInfo = new SessionPositionInfo( @@ -2142,7 +2147,10 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } private static PositionInfo createPositionInfo( - int mediaItemIndex, @Nullable MediaItem mediaItem, long currentPositionMs) { + int mediaItemIndex, + @Nullable MediaItem mediaItem, + long currentPositionMs, + boolean isPlayingAd) { return new PositionInfo( /* windowUid= */ null, /* mediaItemIndex= */ mediaItemIndex, @@ -2151,8 +2159,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; /* periodIndex= */ mediaItemIndex, /* positionMs= */ currentPositionMs, /* contentPositionMs= */ currentPositionMs, - /* adGroupIndex= */ C.INDEX_UNSET, - /* adIndexInAdGroup= */ C.INDEX_UNSET); + /* adGroupIndex= */ isPlayingAd ? 0 : C.INDEX_UNSET, + /* adIndexInAdGroup= */ isPlayingAd ? 0 : C.INDEX_UNSET); } private static SessionPositionInfo createSessionPositionInfo( diff --git a/libraries/session/src/main/java/androidx/media3/session/SessionPositionInfo.java b/libraries/session/src/main/java/androidx/media3/session/SessionPositionInfo.java index 149ef552db..f2464b5af2 100644 --- a/libraries/session/src/main/java/androidx/media3/session/SessionPositionInfo.java +++ b/libraries/session/src/main/java/androidx/media3/session/SessionPositionInfo.java @@ -15,6 +15,7 @@ */ package androidx.media3.session; +import static androidx.media3.common.util.Assertions.checkArgument; import static java.lang.annotation.ElementType.TYPE_USE; import android.os.Bundle; @@ -83,6 +84,7 @@ import java.lang.annotation.Target; long currentLiveOffsetMs, long contentDurationMs, long contentBufferedPositionMs) { + checkArgument(isPlayingAd == (positionInfo.adGroupIndex != C.INDEX_UNSET)); this.positionInfo = positionInfo; this.isPlayingAd = isPlayingAd; this.eventTimeMs = eventTimeMs; diff --git a/libraries/session/src/test/java/androidx/media3/session/SessionPositionInfoTest.java b/libraries/session/src/test/java/androidx/media3/session/SessionPositionInfoTest.java index 57de544dde..9ea9b236b3 100644 --- a/libraries/session/src/test/java/androidx/media3/session/SessionPositionInfoTest.java +++ b/libraries/session/src/test/java/androidx/media3/session/SessionPositionInfoTest.java @@ -18,9 +18,11 @@ package androidx.media3.session; import static com.google.common.truth.Truth.assertThat; import android.os.Bundle; +import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.Player.PositionInfo; import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,7 +36,7 @@ public class SessionPositionInfoTest { new SessionPositionInfo( new PositionInfo( /* windowUid= */ null, - /* windowIndex= */ 33, + /* mediaItemIndex= */ 33, new MediaItem.Builder().setMediaId("1234").build(), /* periodUid= */ null, /* periodIndex= */ 44, @@ -56,4 +58,31 @@ public class SessionPositionInfoTest { SessionPositionInfo.CREATOR.fromBundle(sessionPositionInfoBundle); assertThat(sessionPositionInfo).isEqualTo(testSessionPositionInfo); } + + @Test + public void constructor_invalidIsPlayingAd_throwsIllegalArgumentException() { + Assert.assertThrows( + IllegalArgumentException.class, + () -> + new SessionPositionInfo( + new PositionInfo( + /* windowUid= */ null, + /* mediaItemIndex= */ 33, + MediaItem.EMPTY, + /* periodUid= */ null, + /* periodIndex= */ 44, + /* positionMs= */ 233L, + /* contentPositionMs= */ 333L, + /* adGroupIndex= */ 2, + /* adIndexInAdGroup= */ C.INDEX_UNSET), + /* isPlayingAd= */ false, + /* eventTimeMs= */ 103L, + /* durationMs= */ 400L, + /* bufferedPositionMs= */ 200L, + /* bufferedPercentage= */ 50, + /* totalBufferedDurationMs= */ 500L, + /* currentLiveOffsetMs= */ 20L, + /* contentDurationMs= */ 400L, + /* contentBufferedPositionMs= */ 223L)); + } } diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java index ea40566bb7..7849ffd48b 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java @@ -331,6 +331,7 @@ public class MediaControllerListenerTest { AtomicReference timelineRef = new AtomicReference<>(); AtomicReference playlistMetadataRef = new AtomicReference<>(); AtomicReference audioAttributesRef = new AtomicReference<>(); + AtomicBoolean isPlayingAdRef = new AtomicBoolean(); AtomicInteger currentAdGroupIndexRef = new AtomicInteger(); AtomicInteger currentAdIndexInAdGroupRef = new AtomicInteger(); AtomicBoolean shuffleModeEnabledRef = new AtomicBoolean(); @@ -373,6 +374,7 @@ public class MediaControllerListenerTest { PositionInfo oldPosition, PositionInfo newPosition, @DiscontinuityReason int reason) { + isPlayingAdRef.set(controller.isPlayingAd()); currentAdGroupIndexRef.set(newPosition.adGroupIndex); currentAdIndexInAdGroupRef.set(newPosition.adIndexInAdGroup); latch.countDown(); @@ -398,6 +400,7 @@ public class MediaControllerListenerTest { .setPlaylistMetadata(testPlaylistMetadata) .setShuffleModeEnabled(testShuffleModeEnabled) .setRepeatMode(testRepeatMode) + .setIsPlayingAd(true) .setCurrentAdGroupIndex(testCurrentAdGroupIndex) .setCurrentAdIndexInAdGroup(testCurrentAdIndexInAdGroup) .build(); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java index e077bc2e18..257284b47d 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java @@ -1166,6 +1166,8 @@ public class MediaControllerStateMaskingTest { .setCurrentPosition(initialPosition) .setContentPosition(initialPosition) .setIsPlayingAd(/* isPlayingAd= */ true) + .setCurrentAdGroupIndex(0) + .setCurrentAdIndexInAdGroup(0) .build(); remoteSession.setPlayer(playerConfig); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java index 07cbfff83e..af98cea8ef 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java @@ -899,6 +899,8 @@ public class MediaControllerTest { .setDuration(10_000L) .setIsPlaying(true) .setIsPlayingAd(true) + .setCurrentAdGroupIndex(0) + .setCurrentAdIndexInAdGroup(0) .setPlaybackParameters(new PlaybackParameters(/* speed= */ 2.0f)) .build(); remoteSession.setPlayer(playerConfig);