From cb80fe4c7ccfd2c05fab436affb2f9b45007ee82 Mon Sep 17 00:00:00 2001 From: bachinger Date: Wed, 2 Apr 2025 10:13:23 -0700 Subject: [PATCH] Make AdPlaybackState.withAdDurationsUs work with removed ad groups The assertion is changed to check that the number of passed in arrays of durations is always matching `adGroupCount` according to the behavior of `withRemoveAdGroupCount(int)`. Issue: androidx/media#2267 PiperOrigin-RevId: 743185176 (cherry picked from commit d133300627a3d5757c7dd63fc008b2cf9afc70ba) --- RELEASENOTES.md | 13 +++ .../media3/common/AdPlaybackState.java | 17 ++- .../media3/common/AdPlaybackStateTest.java | 106 ++++++++++++++++++ .../exoplayer/ExoPlayerImplInternal.java | 4 +- .../media3/extractor/mp4/BoxParser.java | 1 - 5 files changed, 134 insertions(+), 7 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 58b27e2e6e..767eb49048 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -13,6 +13,19 @@ * ExoPlayer: * Fix sending `CmcdData` in manifest requests for DASH, HLS, and SmoothStreaming ([#2253](https://github.com/androidx/media/pull/2253)). +<<<<<<< HEAD +======= + * Fix issue where media item transition fails due to recoverable renderer + error during initialization of the next media item + ([#2229](https://github.com/androidx/media/issues/2229)). + * Add `ExoPlayer.setScrubbingModeEnabled(boolean)` method. This optimizes + the player for many frequent seeks (for example, from a user dragging a + scrubber bar around). + * `AdPlaybackState.withAdDurationsUs(long[][])` can be used after ad + groups have been removed. The user still needs to pass in an array of + durations for removed ad groups which can be empty or null + ([#2267](https://github.com/androidx/media/issues/2267)). +>>>>>>> d133300627 (Make AdPlaybackState.withAdDurationsUs work with removed ad groups) * Transformer: * Track Selection: * Extractors: diff --git a/libraries/common/src/main/java/androidx/media3/common/AdPlaybackState.java b/libraries/common/src/main/java/androidx/media3/common/AdPlaybackState.java index 5487d2a8e4..529a30d6bc 100644 --- a/libraries/common/src/main/java/androidx/media3/common/AdPlaybackState.java +++ b/libraries/common/src/main/java/androidx/media3/common/AdPlaybackState.java @@ -1072,14 +1072,23 @@ public final class AdPlaybackState { /** * Returns an instance with the specified ad durations, in microseconds. * - *

Must only be used if {@link #removedAdGroupCount} is 0. + *

The number of arrays of durations ({@code adDurations.length}) must always be equal to + * {@link #adGroupCount}. This is required even on an instance created with {@link + * #withRemovedAdGroupCount(int)}. The array of durations at the index of a removed ad group can + * be null or empty. + * + * @throws IllegalArgumentException if {@code adDurations.length != adGroupCount}. */ @CheckResult public AdPlaybackState withAdDurationsUs(long[][] adDurationUs) { - checkState(removedAdGroupCount == 0); + checkArgument(adDurationUs.length == adGroupCount); AdGroup[] adGroups = Util.nullSafeArrayCopy(this.adGroups, this.adGroups.length); - for (int adGroupIndex = 0; adGroupIndex < adGroupCount; adGroupIndex++) { - adGroups[adGroupIndex] = adGroups[adGroupIndex].withAdDurationsUs(adDurationUs[adGroupIndex]); + for (int correctedAdGroupIndex = 0; + correctedAdGroupIndex < adGroupCount - removedAdGroupCount; + correctedAdGroupIndex++) { + adGroups[correctedAdGroupIndex] = + adGroups[correctedAdGroupIndex].withAdDurationsUs( + adDurationUs[removedAdGroupCount + correctedAdGroupIndex]); } return new AdPlaybackState( adsId, adGroups, adResumePositionUs, contentDurationUs, removedAdGroupCount); diff --git a/libraries/common/src/test/java/androidx/media3/common/AdPlaybackStateTest.java b/libraries/common/src/test/java/androidx/media3/common/AdPlaybackStateTest.java index d98f6eca70..c4908e9f86 100644 --- a/libraries/common/src/test/java/androidx/media3/common/AdPlaybackStateTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/AdPlaybackStateTest.java @@ -1036,4 +1036,110 @@ public class AdPlaybackStateTest { assertThat(AdPlaybackState.AdGroup.fromBundle(adGroup.toBundle()).ids[1]).isNull(); assertThat(AdPlaybackState.AdGroup.fromBundle(adGroup.toBundle())).isEqualTo(adGroup); } + + @Test + public void setDurationsUs_withRemovedAdGroups_updatedCorrectlyAndSafely() { + AdPlaybackState adPlaybackState = + new AdPlaybackState("adsId") + .withLivePostrollPlaceholderAppended(false) + .withNewAdGroup(/* adGroupIndex= */ 0, 10_000) + .withAdCount(/* adGroupIndex= */ 0, 1) + .withAvailableAdMediaItem( + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + MediaItem.fromUri("http://example.com/0-0")) + .withNewAdGroup(/* adGroupIndex= */ 1, 11_000) + .withAdCount(/* adGroupIndex= */ 1, 2) + .withAvailableAdMediaItem( + /* adGroupIndex= */ 1, + /* adIndexInAdGroup= */ 0, + MediaItem.fromUri("http://example.com/1-0")) + .withAvailableAdMediaItem( + /* adGroupIndex= */ 1, + /* adIndexInAdGroup= */ 1, + MediaItem.fromUri("http://example.com/1-1")) + .withNewAdGroup(/* adGroupIndex= */ 2, 12_000) + .withAdCount(/* adGroupIndex= */ 2, 1) + .withAvailableAdMediaItem( + /* adGroupIndex= */ 2, + /* adIndexInAdGroup= */ 0, + MediaItem.fromUri("http://example.com/2-0")); + long[][] adDurationsUs = { + new long[] {10L}, new long[] {20L, 21L}, new long[] {30L}, new long[] {C.TIME_END_OF_SOURCE} + }; + + adPlaybackState = + adPlaybackState + .withAdDurationsUs(adDurationsUs) + .withRemovedAdGroupCount(/* removedAdGroupCount= */ 1); + + assertThat(adPlaybackState.adGroupCount).isEqualTo(4); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).durationsUs).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).count).isEqualTo(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).states).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).isPlaceholder).isFalse(); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).mediaItems).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 0).ids).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 1).durationsUs) + .asList() + .containsExactly(20L, 21L) + .inOrder(); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 2).durationsUs) + .asList() + .containsExactly(30L); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 3).durationsUs) + .asList() + .containsExactly(C.TIME_END_OF_SOURCE); + + adDurationsUs[1][0] = 120L; + adDurationsUs[1][1] = 121L; + adPlaybackState = adPlaybackState.withAdDurationsUs(adDurationsUs); + + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 1).durationsUs) + .asList() + .containsExactly(120L, 121L) + .inOrder(); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 2).durationsUs) + .asList() + .containsExactly(30L); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 3).durationsUs) + .asList() + .containsExactly(C.TIME_END_OF_SOURCE); + + adDurationsUs[0] = null; + adDurationsUs[1] = null; + adDurationsUs[2][0] = C.TIME_UNSET; + adPlaybackState = + adPlaybackState + .withRemovedAdGroupCount(/* removedAdGroupCount= */ 2) + .withAdDurationsUs(adDurationsUs); + + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 1).durationsUs).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 2).durationsUs) + .asList() + .containsExactly(C.TIME_UNSET); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 3).durationsUs) + .asList() + .containsExactly(C.TIME_END_OF_SOURCE); + + adDurationsUs[2] = null; + adDurationsUs[3][0] = 0L; + adPlaybackState = + adPlaybackState + .withRemovedAdGroupCount(/* removedAdGroupCount= */ 3) + .withAdDurationsUs(adDurationsUs); + + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 2).durationsUs).hasLength(0); + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 3).durationsUs) + .asList() + .containsExactly(0L); + + adDurationsUs[3] = null; + adPlaybackState = + adPlaybackState + .withRemovedAdGroupCount(/* removedAdGroupCount= */ 4) + .withAdDurationsUs(adDurationsUs); + + assertThat(adPlaybackState.getAdGroup(/* adGroupIndex= */ 3).durationsUs).hasLength(0); + } } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java index e0038cb0ee..3f29654723 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -751,8 +751,8 @@ import java.util.concurrent.atomic.AtomicBoolean; } } if (e.type == ExoPlaybackException.TYPE_RENDERER - && e.mediaPeriodId != null - && isRendererPrewarmingMediaPeriod(e.rendererIndex, e.mediaPeriodId)) { + && renderers[e.rendererIndex % renderers.length].isRendererPrewarming( + /* id= */ e.rendererIndex)) { // TODO(b/380273486): Investigate recovery for pre-warming renderer errors isPrewarmingDisabledUntilNextTransition = true; disableAndResetPrewarmingRenderers(); diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java index 30d343c10b..0beff55dd5 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java @@ -36,7 +36,6 @@ import androidx.media3.common.util.ParsableBitArray; import androidx.media3.common.util.ParsableByteArray; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; -import androidx.media3.container.DolbyVisionConfig; import androidx.media3.container.Mp4AlternateGroupData; import androidx.media3.container.Mp4Box; import androidx.media3.container.Mp4Box.LeafBox;