From 7899241daf3b53223f3747404451638110669dcf Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 25 Aug 2023 02:03:33 -0700 Subject: [PATCH] Fix period indexing for multi-period DASH DAI live streams The period index was calculated relative to the contentTimeline of the DAI stream, but then used with the full timeline of the player. This means it currently only works when the stream is the only or first item in the playlist. Issue: androidx/media#571 PiperOrigin-RevId: 560023412 --- RELEASENOTES.md | 3 + demos/main/src/main/assets/media.exolist.json | 28 +++++++++ .../ImaServerSideAdInsertionMediaSource.java | 10 +--- .../media3/exoplayer/ima/ImaUtil.java | 24 +++----- .../media3/exoplayer/ima/ImaUtilTest.java | 60 ++++--------------- 5 files changed, 55 insertions(+), 70 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index d70cdedda4..521c467352 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -28,6 +28,9 @@ `VideoFrameProcessor.queueInputBitmap` to `TimestampIterator`. * Muxers: * IMA extension: + * Fix bug where a multi-period DASH live stream that is not the first item + in a playlist can throw an exception + ([#571](https://github.com/androidx/media/issues/571)). * Session: * Set the notifications foreground service behavior to `FOREGROUND_SERVICE_IMMEDIATE` in `DefaultMediaNotificationProvider` diff --git a/demos/main/src/main/assets/media.exolist.json b/demos/main/src/main/assets/media.exolist.json index d7e216f2b9..bd5863b550 100644 --- a/demos/main/src/main/assets/media.exolist.json +++ b/demos/main/src/main/assets/media.exolist.json @@ -474,6 +474,34 @@ "uri": "https://html5demos.com/assets/dizzy.mp4" } ] + }, + { + "name": "Playlist: No ads - DASH live: Tears of Steel (mid) - No ads", + "playlist": [ + { + "uri": "https://html5demos.com/assets/dizzy.mp4" + }, + { + "uri": "ssai://dai.google.com/?assetKey=PSzZMzAkSXCmlJOWDmRj8Q&format=0&adsId=1" + }, + { + "uri": "https://html5demos.com/assets/dizzy.mp4" + } + ] + }, + { + "name": "Playlist: No ads - HLS live: Big Buck Bunny - No ads", + "playlist": [ + { + "uri": "https://html5demos.com/assets/dizzy.mp4" + }, + { + "uri": "ssai://dai.google.com/?assetKey=sN_IYUG8STe1ZzhIIE_ksA&format=2&adsId=3" + }, + { + "uri": "https://html5demos.com/assets/dizzy.mp4" + } + ] } ] }, diff --git a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaServerSideAdInsertionMediaSource.java b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaServerSideAdInsertionMediaSource.java index 7f3e7fdf2d..33623a3b6c 100644 --- a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaServerSideAdInsertionMediaSource.java +++ b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaServerSideAdInsertionMediaSource.java @@ -893,17 +893,13 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou return; } // Map adGroupIndex and adIndexInAdGroup to multi-period window. + int periodIndexInContentTimeline = oldPosition.periodIndex - window.firstPeriodIndex; Pair adGroupIndexAndAdIndexInAdGroup = window.isLive() ? getAdGroupAndIndexInLiveMultiPeriodTimeline( - oldPosition.mediaItemIndex, - oldPosition.periodIndex - window.firstPeriodIndex, - timeline, - adPlaybackState) + periodIndexInContentTimeline, adPlaybackState, checkNotNull(contentTimeline)) : getAdGroupAndIndexInVodMultiPeriodTimeline( - oldPosition.periodIndex - window.firstPeriodIndex, - adPlaybackState, - checkNotNull(contentTimeline)); + periodIndexInContentTimeline, adPlaybackState, checkNotNull(contentTimeline)); adGroupIndex = adGroupIndexAndAdIndexInAdGroup.first; adIndexInAdGroup = adGroupIndexAndAdIndexInAdGroup.second; } diff --git a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaUtil.java b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaUtil.java index 255b90ffd5..2cbb1b4963 100644 --- a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaUtil.java +++ b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/ImaUtil.java @@ -801,22 +801,16 @@ import java.util.Set; * index in the timeline and the {@code adIndexInAdGroup} of the first unplayed ad in that ad * group. * - * @param currentMediaItemIndex The {@linkplain Player#getCurrentMediaItemIndex() current media - * item index}. * @param adPeriodIndex The index of the ad period in the timeline. - * @param timeline The timeline that contains the period for the ad period index. * @param adPlaybackState The ad playback state that holds the ad group and ad information. + * @param timeline The single-window timeline that contains the period for the ad period index. * @return A pair with the ad group index (first) and the ad index in that ad group (second). * @exception IllegalStateException If no unplayed ad is found before or at the start time of the * ad period. */ public static Pair getAdGroupAndIndexInLiveMultiPeriodTimeline( - int currentMediaItemIndex, - int adPeriodIndex, - Timeline timeline, - AdPlaybackState adPlaybackState) { - Timeline.Window window = - timeline.getWindow(/* windowIndex= */ currentMediaItemIndex, new Timeline.Window()); + int adPeriodIndex, AdPlaybackState adPlaybackState, Timeline timeline) { + Timeline.Window window = timeline.getWindow(/* windowIndex= */ 0, new Timeline.Window()); checkArgument(window.isLive()); Timeline.Period period = new Timeline.Period(); timeline.getPeriod(adPeriodIndex, period, /* setIds= */ true); @@ -846,13 +840,13 @@ import java.util.Set; * * @param adPeriodIndex The period index of the ad period. * @param adPlaybackState The ad playback state that holds the ad group and ad information. - * @param contentTimeline The timeline that contains the ad period. + * @param timeline The single-window timeline that contains the ad period. * @return A pair with the ad group index (first) and the ad index in that ad group (second). */ public static Pair getAdGroupAndIndexInVodMultiPeriodTimeline( - int adPeriodIndex, AdPlaybackState adPlaybackState, Timeline contentTimeline) { - Timeline.Window window = contentTimeline.getWindow(/* windowIndex= */ 0, new Timeline.Window()); - checkArgument(contentTimeline.getWindowCount() == 1); + int adPeriodIndex, AdPlaybackState adPlaybackState, Timeline timeline) { + Timeline.Window window = timeline.getWindow(/* windowIndex= */ 0, new Timeline.Window()); + checkArgument(timeline.getWindowCount() == 1); int periodIndex = 0; long totalElapsedContentDurationUs = 0; if (window.isLive()) { @@ -866,8 +860,8 @@ import java.util.Set; AdGroup adGroup = adPlaybackState.getAdGroup(/* adGroupIndex= */ i); long adGroupDurationUs = sum(adGroup.durationsUs); long elapsedAdGroupAdDurationUs = 0; - for (int j = periodIndex; j < min(contentTimeline.getPeriodCount(), adPeriodIndex + 1); j++) { - contentTimeline.getPeriod(j, period, /* setIds= */ true); + for (int j = periodIndex; j < min(timeline.getPeriodCount(), adPeriodIndex + 1); j++) { + timeline.getPeriod(j, period, /* setIds= */ true); if (totalElapsedContentDurationUs < adGroup.timeUs) { // Period starts before the ad group, so it is a content period. totalElapsedContentDurationUs += period.durationUs; diff --git a/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaUtilTest.java b/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaUtilTest.java index e7c694bce5..b119d86998 100644 --- a/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaUtilTest.java +++ b/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaUtilTest.java @@ -2046,10 +2046,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 0, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 0, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 0)); adPlaybackState = @@ -2081,10 +2078,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 2, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 2, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(1, 0)); adPlaybackState = @@ -2092,10 +2086,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 3, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 3, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(1, 1)); adPlaybackState = @@ -2103,10 +2094,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 4, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 4, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(1, 2)); adPlaybackState = @@ -2122,10 +2110,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 6, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 6, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(2, 0)); } @@ -2191,10 +2176,7 @@ public class ImaUtilTest { IllegalStateException.class, () -> getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 1, - contentTimeline, - finalAdPlaybackState)); + /* adPeriodIndex= */ 1, finalAdPlaybackState, contentTimeline)); adPlaybackState = adPlaybackState.withPlayedAd(/* adGroupIndex= */ 1, /* adIndexInAdGroup= */ 0); @@ -2216,10 +2198,7 @@ public class ImaUtilTest { IllegalStateException.class, () -> getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 5, - contentTimeline, - anotherFinalAdPlaybackState)); + /* adPeriodIndex= */ 5, anotherFinalAdPlaybackState, contentTimeline)); } @Test @@ -2275,10 +2254,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 0, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 0, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 2)); } @@ -2330,10 +2306,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 0, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 0, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 1)); // Ad event for second ad in window arrives. @@ -2350,10 +2323,7 @@ public class ImaUtilTest { assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 1, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 1, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 2)); // Move one ad period forward: c, a, a, [a, a, a, a], c @@ -2362,19 +2332,13 @@ public class ImaUtilTest { adPlaybackState.withPlayedAd(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 2); assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 1, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 1, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 3)); adPlaybackState = adPlaybackState.withPlayedAd(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 3); assertThat( getAdGroupAndIndexInLiveMultiPeriodTimeline( - /* currentMediaItemIndex= */ 0, - /* adPeriodIndex= */ 2, - contentTimeline, - adPlaybackState)) + /* adPeriodIndex= */ 2, adPlaybackState, contentTimeline)) .isEqualTo(new Pair<>(0, 4)); }