From 7d5558881df6509801678ab3e21ccb12490ee32e Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 7 May 2019 09:37:33 +0100 Subject: [PATCH] Fix two ad insertion related bugs in DefaultPlaybackSessionManager. 1. A content session after an ad has been played was not re-marked as active, leading to new ad session being marked as active too early. 2. Switching from content to post-roll ended the content session because the return value of getAdGroupTimeUs of C.TIME_END_OF_SOURCE was not handled. Using the nextAdGroupIndex instead. PiperOrigin-RevId: 246977327 --- demos/main/build.gradle | 2 +- .../exoplayer2/demo/TrackSelectionDialog.java | 2 +- .../res/layout/track_selection_dialog.xml | 4 +- .../google/android/exoplayer2/Timeline.java | 3 +- .../DefaultPlaybackSessionManager.java | 16 ++- .../DefaultPlaybackSessionManagerTest.java | 101 ++++++++++++++++++ 6 files changed, 114 insertions(+), 14 deletions(-) diff --git a/demos/main/build.gradle b/demos/main/build.gradle index 7089d4d731..494af011a5 100644 --- a/demos/main/build.gradle +++ b/demos/main/build.gradle @@ -63,7 +63,7 @@ android { dependencies { implementation 'androidx.annotation:annotation:1.0.2' - implementation 'androidx.legacy:legacy-support-core-ui:1.0.0' + implementation 'com.android.support:support-core-ui:' + supportLibraryVersion implementation 'androidx.fragment:fragment:1.0.0' implementation 'com.google.android.material:material:1.0.0' implementation project(modulePrefix + 'library-core') diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/TrackSelectionDialog.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/TrackSelectionDialog.java index a7dd1a0df8..86d01706fb 100644 --- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/TrackSelectionDialog.java +++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/TrackSelectionDialog.java @@ -25,7 +25,7 @@ import androidx.fragment.app.DialogFragment; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; import androidx.fragment.app.FragmentPagerAdapter; -import androidx.viewpager.widget.ViewPager; +import android.support.v4.view.ViewPager; import androidx.appcompat.app.AppCompatDialog; import android.util.SparseArray; import android.view.LayoutInflater; diff --git a/demos/main/src/main/res/layout/track_selection_dialog.xml b/demos/main/src/main/res/layout/track_selection_dialog.xml index 7f6c45e131..24d101ae4c 100644 --- a/demos/main/src/main/res/layout/track_selection_dialog.xml +++ b/demos/main/src/main/res/layout/track_selection_dialog.xml @@ -19,7 +19,7 @@ android:layout_width="match_parent" android:layout_height="match_parent"> - - + adMediaPeriodId.adIndexInAdGroup); } else { - eventTime.timeline.getPeriod(adPeriodIndex, period); - long adGroupTimeMs = - adMediaPeriodId.adGroupIndex < period.getAdGroupCount() - ? C.usToMs(period.getAdGroupTimeUs(adMediaPeriodId.adGroupIndex)) - : 0; // Finished if the event is for content after this ad. - return adGroupTimeMs <= eventTime.currentPlaybackPositionMs; + return eventTime.mediaPeriodId.nextAdGroupIndex == C.INDEX_UNSET + || eventTime.mediaPeriodId.nextAdGroupIndex > adMediaPeriodId.adGroupIndex; } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java index 2993e960b4..f0b18b4a20 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java @@ -598,6 +598,43 @@ public final class DefaultPlaybackSessionManagerTest { assertThat(updatedSessionId300).isEqualTo(sessionId300); } + @Test + public void timelineUpdate_withContent_doesNotFinishFuturePostrollAd() { + Timeline adTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* periodCount= */ 1, + /* id= */ 0, + /* isSeekable= */ true, + /* isDynamic= */ false, + /* durationUs =*/ 10 * C.MICROS_PER_SECOND, + new AdPlaybackState(/* adGroupTimesUs= */ C.TIME_END_OF_SOURCE) + .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1))); + EventTime adEventTime = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + /* windowSequenceNumber= */ 0)); + EventTime contentEventTime = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* windowSequenceNumber= */ 0, + /* nextAdGroupIndex= */ 0)); + sessionManager.updateSessions(contentEventTime); + sessionManager.updateSessions(adEventTime); + + sessionManager.handleTimelineUpdate(contentEventTime); + + verify(mockListener, never()).onSessionFinished(any(), anyString(), anyBoolean()); + } + @Test public void positionDiscontinuity_withinWindow_doesNotFinishSession() { Timeline timeline = @@ -943,6 +980,70 @@ public final class DefaultPlaybackSessionManagerTest { verifyNoMoreInteractions(mockListener); } + @Test + public void + updateSessions_withNewAd_afterDiscontinuitiesFromContentToAdAndBack_doesNotActivateNewAd() { + Timeline adTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* periodCount= */ 1, + /* id= */ 0, + /* isSeekable= */ true, + /* isDynamic= */ false, + /* durationUs =*/ 10 * C.MICROS_PER_SECOND, + new AdPlaybackState( + /* adGroupTimesUs= */ 2 * C.MICROS_PER_SECOND, 5 * C.MICROS_PER_SECOND) + .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) + .withAdCount(/* adGroupIndex= */ 1, /* adCount= */ 1))); + EventTime adEventTime1 = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + /* windowSequenceNumber= */ 0)); + EventTime adEventTime2 = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* adGroupIndex= */ 1, + /* adIndexInAdGroup= */ 0, + /* windowSequenceNumber= */ 0)); + EventTime contentEventTime1 = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* windowSequenceNumber= */ 0, + /* nextAdGroupIndex= */ 0)); + EventTime contentEventTime2 = + createEventTime( + adTimeline, + /* windowIndex= */ 0, + new MediaPeriodId( + adTimeline.getUidOfPeriod(/* periodIndex= */ 0), + /* windowSequenceNumber= */ 0, + /* nextAdGroupIndex= */ 1)); + sessionManager.handleTimelineUpdate(contentEventTime1); + sessionManager.updateSessions(contentEventTime1); + sessionManager.updateSessions(adEventTime1); + sessionManager.handlePositionDiscontinuity( + adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION); + sessionManager.handlePositionDiscontinuity( + contentEventTime2, Player.DISCONTINUITY_REASON_AD_INSERTION); + String adSessionId2 = + sessionManager.getSessionForMediaPeriodId(adTimeline, adEventTime2.mediaPeriodId); + + sessionManager.updateSessions(adEventTime2); + + verify(mockListener, never()).onSessionActive(any(), eq(adSessionId2)); + } + private static EventTime createEventTime( Timeline timeline, int windowIndex, @Nullable MediaPeriodId mediaPeriodId) { return new EventTime(