From 21996be448e5c333886cf135d8ba75502db20be6 Mon Sep 17 00:00:00 2001 From: rohks Date: Wed, 4 Jan 2023 17:55:58 +0000 Subject: [PATCH] Optimise bundling for `Timeline.Window` and `Timeline.Period` Improves the time taken to construct playerInfo from its bundle from ~400 ms to ~300 ms. Also made `Timeline.Window.toBundle(boolean excludeMediaItem)` public as it was required to assert a condition in tests. PiperOrigin-RevId: 499512353 (cherry picked from commit 790e27d929906a36438af5b42ef62a13e4719045) --- .../java/androidx/media3/common/Timeline.java | 91 ++++++++++++++----- .../androidx/media3/common/TimelineTest.java | 50 +++++++++- 2 files changed, 116 insertions(+), 25 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/Timeline.java b/libraries/common/src/main/java/androidx/media3/common/Timeline.java index 679df19aae..d95b27f2bc 100644 --- a/libraries/common/src/main/java/androidx/media3/common/Timeline.java +++ b/libraries/common/src/main/java/androidx/media3/common/Timeline.java @@ -454,27 +454,60 @@ public abstract class Timeline implements Bundleable { private static final int FIELD_LAST_PERIOD_INDEX = 12; private static final int FIELD_POSITION_IN_FIRST_PERIOD_US = 13; - private final Bundle toBundle(boolean excludeMediaItem) { + /** + * Returns a {@link Bundle} representing the information stored in this object. + * + *

It omits the {@link #uid} and {@link #manifest} fields. The {@link #uid} of an instance + * restored by {@link #CREATOR} will be a fake {@link Object} and the {@link #manifest} of the + * instance will be {@code null}. + * + * @param excludeMediaItem Whether to exclude {@link #mediaItem} of window. + */ + @UnstableApi + public Bundle toBundle(boolean excludeMediaItem) { Bundle bundle = new Bundle(); - bundle.putBundle( - keyForField(FIELD_MEDIA_ITEM), - excludeMediaItem ? MediaItem.EMPTY.toBundle() : mediaItem.toBundle()); - bundle.putLong(keyForField(FIELD_PRESENTATION_START_TIME_MS), presentationStartTimeMs); - bundle.putLong(keyForField(FIELD_WINDOW_START_TIME_MS), windowStartTimeMs); - bundle.putLong( - keyForField(FIELD_ELAPSED_REALTIME_EPOCH_OFFSET_MS), elapsedRealtimeEpochOffsetMs); - bundle.putBoolean(keyForField(FIELD_IS_SEEKABLE), isSeekable); - bundle.putBoolean(keyForField(FIELD_IS_DYNAMIC), isDynamic); + if (!excludeMediaItem) { + bundle.putBundle(keyForField(FIELD_MEDIA_ITEM), mediaItem.toBundle()); + } + if (presentationStartTimeMs != C.TIME_UNSET) { + bundle.putLong(keyForField(FIELD_PRESENTATION_START_TIME_MS), presentationStartTimeMs); + } + if (windowStartTimeMs != C.TIME_UNSET) { + bundle.putLong(keyForField(FIELD_WINDOW_START_TIME_MS), windowStartTimeMs); + } + if (elapsedRealtimeEpochOffsetMs != C.TIME_UNSET) { + bundle.putLong( + keyForField(FIELD_ELAPSED_REALTIME_EPOCH_OFFSET_MS), elapsedRealtimeEpochOffsetMs); + } + if (isSeekable) { + bundle.putBoolean(keyForField(FIELD_IS_SEEKABLE), isSeekable); + } + if (isDynamic) { + bundle.putBoolean(keyForField(FIELD_IS_DYNAMIC), isDynamic); + } + @Nullable MediaItem.LiveConfiguration liveConfiguration = this.liveConfiguration; if (liveConfiguration != null) { bundle.putBundle(keyForField(FIELD_LIVE_CONFIGURATION), liveConfiguration.toBundle()); } - bundle.putBoolean(keyForField(FIELD_IS_PLACEHOLDER), isPlaceholder); - bundle.putLong(keyForField(FIELD_DEFAULT_POSITION_US), defaultPositionUs); - bundle.putLong(keyForField(FIELD_DURATION_US), durationUs); - bundle.putInt(keyForField(FIELD_FIRST_PERIOD_INDEX), firstPeriodIndex); - bundle.putInt(keyForField(FIELD_LAST_PERIOD_INDEX), lastPeriodIndex); - bundle.putLong(keyForField(FIELD_POSITION_IN_FIRST_PERIOD_US), positionInFirstPeriodUs); + if (isPlaceholder) { + bundle.putBoolean(keyForField(FIELD_IS_PLACEHOLDER), isPlaceholder); + } + if (defaultPositionUs != 0) { + bundle.putLong(keyForField(FIELD_DEFAULT_POSITION_US), defaultPositionUs); + } + if (durationUs != C.TIME_UNSET) { + bundle.putLong(keyForField(FIELD_DURATION_US), durationUs); + } + if (firstPeriodIndex != 0) { + bundle.putInt(keyForField(FIELD_FIRST_PERIOD_INDEX), firstPeriodIndex); + } + if (lastPeriodIndex != 0) { + bundle.putInt(keyForField(FIELD_LAST_PERIOD_INDEX), lastPeriodIndex); + } + if (positionInFirstPeriodUs != 0) { + bundle.putLong(keyForField(FIELD_POSITION_IN_FIRST_PERIOD_US), positionInFirstPeriodUs); + } return bundle; } @@ -504,7 +537,7 @@ public abstract class Timeline implements Bundleable { @Nullable Bundle mediaItemBundle = bundle.getBundle(keyForField(FIELD_MEDIA_ITEM)); @Nullable MediaItem mediaItem = - mediaItemBundle != null ? MediaItem.CREATOR.fromBundle(mediaItemBundle) : null; + mediaItemBundle != null ? MediaItem.CREATOR.fromBundle(mediaItemBundle) : MediaItem.EMPTY; long presentationStartTimeMs = bundle.getLong( keyForField(FIELD_PRESENTATION_START_TIME_MS), /* defaultValue= */ C.TIME_UNSET); @@ -936,16 +969,25 @@ public abstract class Timeline implements Bundleable { *

It omits the {@link #id} and {@link #uid} fields so these fields of an instance restored * by {@link #CREATOR} will always be {@code null}. */ - // TODO(b/166765820): See if missing fields would be okay and add them to the Bundle otherwise. @UnstableApi @Override public Bundle toBundle() { Bundle bundle = new Bundle(); - bundle.putInt(keyForField(FIELD_WINDOW_INDEX), windowIndex); - bundle.putLong(keyForField(FIELD_DURATION_US), durationUs); - bundle.putLong(keyForField(FIELD_POSITION_IN_WINDOW_US), positionInWindowUs); - bundle.putBoolean(keyForField(FIELD_PLACEHOLDER), isPlaceholder); - bundle.putBundle(keyForField(FIELD_AD_PLAYBACK_STATE), adPlaybackState.toBundle()); + if (windowIndex != 0) { + bundle.putInt(keyForField(FIELD_WINDOW_INDEX), windowIndex); + } + if (durationUs != C.TIME_UNSET) { + bundle.putLong(keyForField(FIELD_DURATION_US), durationUs); + } + if (positionInWindowUs != 0) { + bundle.putLong(keyForField(FIELD_POSITION_IN_WINDOW_US), positionInWindowUs); + } + if (isPlaceholder) { + bundle.putBoolean(keyForField(FIELD_PLACEHOLDER), isPlaceholder); + } + if (!adPlaybackState.equals(AdPlaybackState.NONE)) { + bundle.putBundle(keyForField(FIELD_AD_PLAYBACK_STATE), adPlaybackState.toBundle()); + } return bundle; } @@ -962,7 +1004,8 @@ public abstract class Timeline implements Bundleable { bundle.getLong(keyForField(FIELD_DURATION_US), /* defaultValue= */ C.TIME_UNSET); long positionInWindowUs = bundle.getLong(keyForField(FIELD_POSITION_IN_WINDOW_US), /* defaultValue= */ 0); - boolean isPlaceholder = bundle.getBoolean(keyForField(FIELD_PLACEHOLDER)); + boolean isPlaceholder = + bundle.getBoolean(keyForField(FIELD_PLACEHOLDER), /* defaultValue= */ false); @Nullable Bundle adPlaybackStateBundle = bundle.getBundle(keyForField(FIELD_AD_PLAYBACK_STATE)); AdPlaybackState adPlaybackState = diff --git a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java index 6844330e14..111652b38b 100644 --- a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java @@ -17,6 +17,7 @@ package androidx.media3.common; import static com.google.common.truth.Truth.assertThat; +import android.os.Bundle; import androidx.annotation.Nullable; import androidx.media3.common.MediaItem.LiveConfiguration; import androidx.media3.exoplayer.source.ShuffleOrder.DefaultShuffleOrder; @@ -267,7 +268,9 @@ public class TimelineTest { /* durationUs= */ 2, /* defaultPositionUs= */ 22, /* windowOffsetInFirstPeriodUs= */ 222, - ImmutableList.of(AdPlaybackState.NONE), + ImmutableList.of( + new AdPlaybackState( + /* adsId= */ null, /* adGroupTimesUs...= */ 10_000, 20_000)), new MediaItem.Builder().setMediaId("mediaId2").build()), new TimelineWindowDefinition( /* periodCount= */ 3, @@ -334,6 +337,31 @@ public class TimelineTest { TimelineAsserts.assertEmpty(Timeline.CREATOR.fromBundle(Timeline.EMPTY.toBundle())); } + @Test + public void window_toBundleSkipsDefaultValues_fromBundleRestoresThem() { + Timeline.Window window = new Timeline.Window(); + // Please refrain from altering these default values since doing so would cause issues with + // backwards compatibility. + window.presentationStartTimeMs = C.TIME_UNSET; + window.windowStartTimeMs = C.TIME_UNSET; + window.elapsedRealtimeEpochOffsetMs = C.TIME_UNSET; + window.durationUs = C.TIME_UNSET; + window.mediaItem = new MediaItem.Builder().build(); + + Bundle windowBundle = window.toBundle(); + + // Check that default values are skipped when bundling. MediaItem key is not added to the bundle + // only when excludeMediaItem is true. + assertThat(windowBundle.keySet()).hasSize(1); + assertThat(window.toBundle(/* excludeMediaItem= */ true).keySet()).isEmpty(); + + Timeline.Window restoredWindow = Timeline.Window.CREATOR.fromBundle(windowBundle); + + assertThat(restoredWindow.manifest).isNull(); + TimelineAsserts.assertWindowEqualsExceptUidAndManifest( + /* expectedWindow= */ window, /* actualWindow= */ restoredWindow); + } + @Test public void roundTripViaBundle_ofWindow_yieldsEqualInstanceExceptUidAndManifest() { Timeline.Window window = new Timeline.Window(); @@ -367,6 +395,26 @@ public class TimelineTest { /* expectedWindow= */ window, /* actualWindow= */ restoredWindow); } + @Test + public void period_toBundleSkipsDefaultValues_fromBundleRestoresThem() { + Timeline.Period period = new Timeline.Period(); + // Please refrain from altering these default values since doing so would cause issues with + // backwards compatibility. + period.durationUs = C.TIME_UNSET; + + Bundle periodBundle = period.toBundle(); + + // Check that default values are skipped when bundling. + assertThat(periodBundle.keySet()).isEmpty(); + + Timeline.Period restoredPeriod = Timeline.Period.CREATOR.fromBundle(periodBundle); + + assertThat(restoredPeriod.id).isNull(); + assertThat(restoredPeriod.uid).isNull(); + TimelineAsserts.assertPeriodEqualsExceptIds( + /* expectedPeriod= */ period, /* actualPeriod= */ restoredPeriod); + } + @Test public void roundTripViaBundle_ofPeriod_yieldsEqualInstanceExceptIds() { Timeline.Period period = new Timeline.Period();