From 24ca6828ebd0f8c9d32f796822956592ce9cec83 Mon Sep 17 00:00:00 2001 From: bachinger Date: Thu, 6 Apr 2023 16:16:28 +0100 Subject: [PATCH] Do not select unprepared media period in getMediaPeriodForEvent There is a race with the ad period preparation having completed and `onDownstreamFormatChanged` being called when a live stream is joined in an ad period. In this case the stream event metadata of the period is immediately emitted and causing an ad media period being created that is selected in `getMediaPeriodForEvent` before being prepared (1 out of 4). Using an `isPrepared` flag makes sure we don't hand out the media period to early in `getMediaPeriodForEvent`. PiperOrigin-RevId: 522340046 --- .../ads/ServerSideAdInsertionMediaSource.java | 19 ++- .../ServerSideAdInsertionMediaSourceTest.java | 130 ++++++++++++++++++ 2 files changed, 144 insertions(+), 5 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java index a210cc689b..b7ad76f6d7 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java @@ -688,6 +688,9 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource if (mediaLoadData != null && mediaLoadData.mediaStartTimeMs != C.TIME_UNSET) { for (int i = 0; i < mediaPeriods.size(); i++) { MediaPeriodImpl mediaPeriod = mediaPeriods.get(i); + if (!mediaPeriod.isPrepared) { + continue; + } long startTimeInPeriodUs = getMediaPeriodPositionUs( Util.msToUs(mediaLoadData.mediaStartTimeMs), @@ -706,7 +709,7 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource mediaPeriod.lastStartPositionUs = positionUs; if (hasStartedPreparing) { if (isPrepared) { - checkNotNull(mediaPeriod.callback).onPrepared(mediaPeriod); + mediaPeriod.onPrepared(); } return; } @@ -923,10 +926,7 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource public void onPrepared(MediaPeriod actualMediaPeriod) { isPrepared = true; for (int i = 0; i < mediaPeriods.size(); i++) { - MediaPeriodImpl mediaPeriod = mediaPeriods.get(i); - if (mediaPeriod.callback != null) { - mediaPeriod.callback.onPrepared(mediaPeriod); - } + mediaPeriods.get(i).onPrepared(); } } @@ -1104,6 +1104,7 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource public @MonotonicNonNull Callback callback; public long lastStartPositionUs; public boolean[] hasNotifiedDownstreamFormatChange; + public boolean isPrepared; public MediaPeriodImpl( SharedMediaPeriod sharedPeriod, @@ -1117,6 +1118,14 @@ public final class ServerSideAdInsertionMediaSource extends BaseMediaSource hasNotifiedDownstreamFormatChange = new boolean[0]; } + /** Called when the preparation has completed. */ + public void onPrepared() { + if (callback != null) { + callback.onPrepared(this); + } + isPrepared = true; + } + @Override public void prepare(Callback callback, long positionUs) { this.callback = callback; diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSourceTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSourceTest.java index 0c77095421..ba9935a2fa 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSourceTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSourceTest.java @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer.source.ads; +import static androidx.media3.common.C.DATA_TYPE_MEDIA; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.exoplayer.source.ads.ServerSideAdInsertionUtil.addAdGroupToAdPlaybackState; import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil; @@ -33,18 +34,28 @@ import static org.mockito.Mockito.verify; import android.content.Context; import android.graphics.SurfaceTexture; +import android.os.Handler; import android.util.Pair; import android.view.Surface; +import androidx.annotation.Nullable; import androidx.media3.common.AdPlaybackState; import androidx.media3.common.C; +import androidx.media3.common.Format; import androidx.media3.common.MediaItem; import androidx.media3.common.Player; import androidx.media3.common.Timeline; +import androidx.media3.common.util.Clock; +import androidx.media3.common.util.Util; import androidx.media3.exoplayer.ExoPlayer; import androidx.media3.exoplayer.analytics.AnalyticsListener; import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.source.DefaultMediaSourceFactory; +import androidx.media3.exoplayer.source.MediaLoadData; +import androidx.media3.exoplayer.source.MediaPeriod; +import androidx.media3.exoplayer.source.MediaSource; +import androidx.media3.exoplayer.source.MediaSourceEventListener; import androidx.media3.exoplayer.source.SinglePeriodTimeline; +import androidx.media3.exoplayer.upstream.DefaultAllocator; import androidx.media3.test.utils.CapturingRenderersFactory; import androidx.media3.test.utils.DumpFileAsserts; import androidx.media3.test.utils.FakeClock; @@ -151,6 +162,125 @@ public final class ServerSideAdInsertionMediaSourceTest { assertThat(window.durationUs).isEqualTo(9_800_000); } + @Test + public void createPeriod_unpreparedAdMediaPeriodImplReplacesContentPeriod_adPeriodNotSelected() + throws Exception { + DefaultAllocator allocator = + new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024); + MediaPeriod.Callback callback = + new MediaPeriod.Callback() { + @Override + public void onPrepared(MediaPeriod mediaPeriod) {} + + @Override + public void onContinueLoadingRequested(MediaPeriod source) {} + }; + AdPlaybackState adPlaybackState = + new AdPlaybackState("adsId").withLivePostrollPlaceholderAppended(); + FakeTimeline wrappedTimeline = + new FakeTimeline( + new FakeTimeline.TimelineWindowDefinition( + /* periodCount= */ 1, + /* id= */ 0, + /* isSeekable= */ true, + /* isDynamic= */ false, + /* isLive= */ false, + /* isPlaceholder= */ false, + /* durationUs= */ 10_000_000L, + /* defaultPositionUs= */ 3_000_000L, + /* windowOffsetInFirstPeriodUs= */ 0L, + AdPlaybackState.NONE)); + ServerSideAdInsertionMediaSource mediaSource = + new ServerSideAdInsertionMediaSource( + new FakeMediaSource(wrappedTimeline), /* adPlaybackStateUpdater= */ null); + AtomicReference timelineReference = new AtomicReference<>(); + AtomicReference mediaPeriodIdReference = new AtomicReference<>(); + mediaSource.setAdPlaybackStates( + ImmutableMap.of(new Pair<>(0, 0), adPlaybackState), wrappedTimeline); + mediaSource.addEventListener( + new Handler(Util.getCurrentOrMainLooper()), + new MediaSourceEventListener() { + @Override + public void onDownstreamFormatChanged( + int windowIndex, + @Nullable MediaSource.MediaPeriodId mediaPeriodId, + MediaLoadData mediaLoadData) { + mediaPeriodIdReference.set(mediaPeriodId); + } + }); + mediaSource.prepareSource( + (source, timeline) -> timelineReference.set(timeline), + /* mediaTransferListener= */ null, + PlayerId.UNSET); + runMainLooperUntil(() -> timelineReference.get() != null); + Timeline firstTimeline = timelineReference.get(); + MediaSource.MediaPeriodId mediaPeriodId1 = + new MediaSource.MediaPeriodId( + new Pair<>(0, 0), /* windowSequenceNumber= */ 0L, /* nextAdGroupIndex= */ 0); + MediaSource.MediaPeriodId mediaPeriodId2 = + new MediaSource.MediaPeriodId( + new Pair<>(0, 0), + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + /* windowSequenceNumber= */ 0L); + + // Create and prepare the first period. + MediaPeriod mediaPeriod1 = + mediaSource.createPeriod(mediaPeriodId1, allocator, /* startPositionUs= */ 0L); + mediaPeriod1.prepare(callback, /* positionUs= */ 0L); + + // Update the playback state to turn the content period into an ad period. + adPlaybackState = + adPlaybackState + .withNewAdGroup(/* adGroupIndex= */ 0, /* adGroupTimeUs= */ 0L) + .withIsServerSideInserted(/* adGroupIndex= */ 0, true) + .withAdCount(/* adGroupIndex= */ 0, 1) + .withContentResumeOffsetUs(/* adGroupIndex= */ 0, 10_000_000L) + .withAdDurationsUs(/* adGroupIndex= */ 0, 10_000_000L); + mediaSource.setAdPlaybackStates( + ImmutableMap.of(new Pair<>(0, 0), adPlaybackState), wrappedTimeline); + runMainLooperUntil(() -> !timelineReference.get().equals(firstTimeline)); + + // Create the second period that is tied to the same SharedMediaPeriod internally. + mediaSource.createPeriod(mediaPeriodId2, allocator, /* startPositionUs= */ 0L); + + // Issue a onDownstreamFormatChanged event for mediaPeriodId1. The SharedPeriod selects in + // `getMediaPeriodForEvent` from the following `MediaPeriodImpl`s for + // MediaLoadData.mediaStartTimeMs=0 to 10_000_00. + // [ + // isPrepared: true, + // startPositionMs: 0, + // endPositionMs: 0, + // adGroupIndex: -1, + // adIndexInAdGroup: -1, + // nextAdGroupIndex: 0, + // ], + // [ + // isPrepared: false, + // startPositionMs: 0, + // endPositionMs: 10_000_000, + // adGroupIndex: 0, + // adIndexInAdGroup: 0, + // nextAdGroupIndex: -1, + // ] + MediaLoadData mediaLoadData = + new MediaLoadData( + /* dataType= */ DATA_TYPE_MEDIA, + C.TRACK_TYPE_VIDEO, + new Format.Builder().build(), + C.SELECTION_REASON_INITIAL, + /* trackSelectionData= */ null, + /* mediaStartTimeMs= */ 123L, + /* mediaEndTimeMs= */ 10_000_000L); + mediaSource.onDownstreamFormatChanged(/* windowIndex= */ 0, mediaPeriodId1, mediaLoadData); + runMainLooperUntil( + () -> mediaPeriodId1.equals(mediaPeriodIdReference.get()), + /* timeoutMs= */ 500L, + Clock.DEFAULT); + + assertThat(mediaPeriodIdReference.get()).isEqualTo(mediaPeriodId1); + } + @Test public void timeline_liveSinglePeriodWithUnsetPeriodDuration_containsAdsDefinedInAdPlaybackState() throws Exception {