Remove assertion that prevents masking of ad periods

The assertion asserts against a `Period` and an `AdPlaybackState` which actually asserts against a resolved ad which is what `ExoPlayerImplInternal` does later and what gives us a `SEEK_ADJUSTMENT`. However, this assertion is not required at the moment of masking, because we are sure that the resolved seek results in a content period and never an ad period. #minor-release Issue: androidx/media#122 PiperOrigin-RevId: 471827072 (cherry picked from commit 73f86682e94024c780564dbc287f06d47dfeca26)
2025-05-11 17:49:52 +08:00 · 2022-09-02 17:05:21 +00:00 · 2022-09-02 17:05:21 +00:00 · 1b7060776b
commit 1b7060776b
parent 2dfca4fca8
8 changed files with 60 additions and 16 deletions
--- 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
--- 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) {
--- 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(
--- 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;
--- 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));
+  }
 }
--- 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<Timeline> timelineRef = new AtomicReference<>();
    AtomicReference<MediaMetadata> playlistMetadataRef = new AtomicReference<>();
    AtomicReference<AudioAttributes> 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();
--- 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);

--- 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);