Deprecate setTag parameter in Timeline.getWindow.

There is no point in having this parameter as the tag should always be a single
immutable object instantiated at the time the Timeline is created or earlier.

So there is no preformance benefit and it's error-prone in case people
forget to set setTag=true.

PiperOrigin-RevId: 264117041
This commit is contained in:
tonihei 2019-08-19 10:50:22 +01:00 committed by Toni
parent 652c2f9c18
commit 20fd4e16d2
14 changed files with 43 additions and 66 deletions

View file

@ -40,6 +40,7 @@
([#5619](https://github.com/google/ExoPlayer/issues/5619)).
* Fix issue where player errors are thrown too early at playlist transitions
([#5407](https://github.com/google/ExoPlayer/issues/5407)).
* Deprecate `setTag` parameter of `Timeline.getWindow`. Tags will always be set.
### 2.10.4 ###

View file

@ -110,13 +110,11 @@ import java.util.Arrays;
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
long durationUs = durationsUs[windowIndex];
boolean isDynamic = durationUs == C.TIME_UNSET;
Object tag = setTag ? ids[windowIndex] : null;
return window.set(
tag,
/* tag= */ ids[windowIndex],
/* manifest= */ null,
/* presentationStartTimeMs= */ C.TIME_UNSET,
/* windowStartTimeMs= */ C.TIME_UNSET,

View file

@ -95,18 +95,14 @@ public abstract class BasePlayer implements Player {
@Nullable
public final Object getCurrentTag() {
Timeline timeline = getCurrentTimeline();
return timeline.isEmpty()
? null
: timeline.getWindow(getCurrentWindowIndex(), window, /* setTag= */ true).tag;
return timeline.isEmpty() ? null : timeline.getWindow(getCurrentWindowIndex(), window).tag;
}
@Override
@Nullable
public final Object getCurrentManifest() {
Timeline timeline = getCurrentTimeline();
return timeline.isEmpty()
? null
: timeline.getWindow(getCurrentWindowIndex(), window, /* setTag= */ false).manifest;
return timeline.isEmpty() ? null : timeline.getWindow(getCurrentWindowIndex(), window).manifest;
}
@Override

View file

@ -520,8 +520,7 @@ public abstract class Timeline {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
throw new IndexOutOfBoundsException();
}
@ -633,28 +632,20 @@ public abstract class Timeline {
}
/**
* Populates a {@link Window} with data for the window at the specified index. Does not populate
* {@link Window#tag}.
* Populates a {@link Window} with data for the window at the specified index.
*
* @param windowIndex The index of the window.
* @param window The {@link Window} to populate. Must not be null.
* @return The populated {@link Window}, for convenience.
*/
public final Window getWindow(int windowIndex, Window window) {
return getWindow(windowIndex, window, false);
return getWindow(windowIndex, window, /* defaultPositionProjectionUs= */ 0);
}
/**
* Populates a {@link Window} with data for the window at the specified index.
*
* @param windowIndex The index of the window.
* @param window The {@link Window} to populate. Must not be null.
* @param setTag Whether {@link Window#tag} should be populated. If false, the field will be set
* to null. The caller should pass false for efficiency reasons unless the field is required.
* @return The populated {@link Window}, for convenience.
*/
/** @deprecated Use {@link #getWindow(int, Window)} instead. Tags will always be set. */
@Deprecated
public final Window getWindow(int windowIndex, Window window, boolean setTag) {
return getWindow(windowIndex, window, setTag, 0);
return getWindow(windowIndex, window, /* defaultPositionProjectionUs= */ 0);
}
/**
@ -662,14 +653,21 @@ public abstract class Timeline {
*
* @param windowIndex The index of the window.
* @param window The {@link Window} to populate. Must not be null.
* @param setTag Whether {@link Window#tag} should be populated. If false, the field will be set
* to null. The caller should pass false for efficiency reasons unless the field is required.
* @param defaultPositionProjectionUs A duration into the future that the populated window's
* default start position should be projected.
* @return The populated {@link Window}, for convenience.
*/
public abstract Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs);
@SuppressWarnings("deprecation")
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
return getWindow(windowIndex, window, /* setTag= */ true, defaultPositionProjectionUs);
}
/** @deprecated Implement {@link #getWindow(int, Window, long)} instead and always set the tag. */
@Deprecated
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
return getWindow(windowIndex, window, defaultPositionProjectionUs);
}
/**
* Returns the number of periods in the timeline.
@ -750,7 +748,7 @@ public abstract class Timeline {
long windowPositionUs,
long defaultPositionProjectionUs) {
Assertions.checkIndex(windowIndex, 0, getWindowCount());
getWindow(windowIndex, window, false, defaultPositionProjectionUs);
getWindow(windowIndex, window, defaultPositionProjectionUs);
if (windowPositionUs == C.TIME_UNSET) {
windowPositionUs = window.getDefaultPositionUs();
if (windowPositionUs == C.TIME_UNSET) {

View file

@ -189,14 +189,12 @@ import com.google.android.exoplayer2.util.Assertions;
}
@Override
public final Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public final Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
int childIndex = getChildIndexByWindowIndex(windowIndex);
int firstWindowIndexInChild = getFirstWindowIndexByChildIndex(childIndex);
int firstPeriodIndexInChild = getFirstPeriodIndexByChildIndex(childIndex);
getTimelineByChildIndex(childIndex)
.getWindow(
windowIndex - firstWindowIndexInChild, window, setTag, defaultPositionProjectionUs);
.getWindow(windowIndex - firstWindowIndexInChild, window, defaultPositionProjectionUs);
window.firstPeriodIndex += firstPeriodIndexInChild;
window.lastPeriodIndex += firstPeriodIndexInChild;
return window;

View file

@ -342,10 +342,8 @@ public final class ClippingMediaSource extends CompositeMediaSource<Void> {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
timeline.getWindow(
/* windowIndex= */ 0, window, setTag, /* defaultPositionProjectionUs= */ 0);
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
timeline.getWindow(/* windowIndex= */ 0, window, /* defaultPositionProjectionUs= */ 0);
window.positionInFirstPeriodUs += startUs;
window.durationUs = durationUs;
window.isDynamic = isDynamic;

View file

@ -57,9 +57,8 @@ public abstract class ForwardingTimeline extends Timeline {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
return timeline.getWindow(windowIndex, window, setTag, defaultPositionProjectionUs);
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
return timeline.getWindow(windowIndex, window, defaultPositionProjectionUs);
}
@Override

View file

@ -287,8 +287,7 @@ public final class MaskingMediaSource extends CompositeMediaSource<Void> {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
return window.set(
tag,
/* manifest= */ null,

View file

@ -159,10 +159,8 @@ public final class SinglePeriodTimeline extends Timeline {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
Assertions.checkIndex(windowIndex, 0, 1);
Object tag = setTag ? this.tag : null;
long windowDefaultStartPositionUs = this.windowDefaultStartPositionUs;
if (isDynamic && defaultPositionProjectionUs != 0) {
if (windowDurationUs == C.TIME_UNSET) {

View file

@ -55,9 +55,8 @@ public final class SinglePeriodAdTimeline extends ForwardingTimeline {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
window = super.getWindow(windowIndex, window, setTag, defaultPositionProjectionUs);
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
window = super.getWindow(windowIndex, window, defaultPositionProjectionUs);
if (window.durationUs == C.TIME_UNSET) {
window.durationUs = adPlaybackState.contentDurationUs;
}

View file

@ -88,8 +88,7 @@ public final class SinglePeriodTimelineTest {
/* manifest= */ null,
/* tag= */ null);
assertThat(timeline.getWindow(/* windowIndex= */ 0, window, /* setTag= */ false).tag).isNull();
assertThat(timeline.getWindow(/* windowIndex= */ 0, window, /* setTag= */ true).tag).isNull();
assertThat(timeline.getWindow(/* windowIndex= */ 0, window).tag).isNull();
assertThat(timeline.getPeriod(/* periodIndex= */ 0, period, /* setIds= */ false).id).isNull();
assertThat(timeline.getPeriod(/* periodIndex= */ 0, period, /* setIds= */ true).id).isNull();
assertThat(timeline.getPeriod(/* periodIndex= */ 0, period, /* setIds= */ false).uid).isNull();
@ -98,7 +97,7 @@ public final class SinglePeriodTimelineTest {
}
@Test
public void setTag_isUsedForWindowTag() {
public void getWindow_setsTag() {
Object tag = new Object();
SinglePeriodTimeline timeline =
new SinglePeriodTimeline(
@ -108,9 +107,7 @@ public final class SinglePeriodTimelineTest {
/* manifest= */ null,
tag);
assertThat(timeline.getWindow(/* windowIndex= */ 0, window, /* setTag= */ false).tag).isNull();
assertThat(timeline.getWindow(/* windowIndex= */ 0, window, /* setTag= */ true).tag)
.isEqualTo(tag);
assertThat(timeline.getWindow(/* windowIndex= */ 0, window).tag).isEqualTo(tag);
}
@Test

View file

@ -1207,18 +1207,16 @@ public final class DashMediaSource extends BaseMediaSource {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
Assertions.checkIndex(windowIndex, 0, 1);
long windowDefaultStartPositionUs = getAdjustedWindowDefaultStartPositionUs(
defaultPositionProjectionUs);
Object tag = setTag ? windowTag : null;
boolean isDynamic =
manifest.dynamic
&& manifest.minUpdatePeriodMs != C.TIME_UNSET
&& manifest.durationMs == C.TIME_UNSET;
return window.set(
tag,
windowTag,
manifest,
presentationStartTimeMs,
windowStartTimeMs,

View file

@ -179,12 +179,10 @@ public final class FakeTimeline extends Timeline {
}
@Override
public Window getWindow(
int windowIndex, Window window, boolean setTag, long defaultPositionProjectionUs) {
public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) {
TimelineWindowDefinition windowDefinition = windowDefinitions[windowIndex];
Object tag = setTag ? windowDefinition.id : null;
return window.set(
tag,
/* tag= */ windowDefinition.id,
manifests[windowIndex],
/* presentationStartTimeMs= */ C.TIME_UNSET,
/* windowStartTimeMs= */ C.TIME_UNSET,

View file

@ -52,7 +52,7 @@ public final class TimelineAsserts {
Window window = new Window();
assertThat(timeline.getWindowCount()).isEqualTo(expectedWindowTags.length);
for (int i = 0; i < timeline.getWindowCount(); i++) {
timeline.getWindow(i, window, true);
timeline.getWindow(i, window);
if (expectedWindowTags[i] != null) {
assertThat(window.tag).isEqualTo(expectedWindowTags[i]);
}
@ -63,7 +63,7 @@ public final class TimelineAsserts {
public static void assertWindowIsDynamic(Timeline timeline, boolean... windowIsDynamic) {
Window window = new Window();
for (int i = 0; i < timeline.getWindowCount(); i++) {
timeline.getWindow(i, window, true);
timeline.getWindow(i, window);
assertThat(window.isDynamic).isEqualTo(windowIsDynamic[i]);
}
}
@ -129,7 +129,7 @@ public final class TimelineAsserts {
Window window = new Window();
Period period = new Period();
for (int i = 0; i < windowCount; i++) {
timeline.getWindow(i, window, true);
timeline.getWindow(i, window);
assertThat(window.firstPeriodIndex).isEqualTo(accumulatedPeriodCounts[i]);
assertThat(window.lastPeriodIndex).isEqualTo(accumulatedPeriodCounts[i + 1] - 1);
}