Skip to content

Commit

Permalink
Add fail-early checks for TrackSelectorResult correctness
Browse files Browse the repository at this point in the history
The two arrays need to have the same length and the selection
must match in their nullness (unless for TYPE_NONE
renderers). Clarify this more clearly in the docs and add
new asssertions for it. This avoids that the player is failing
in obscure ways much later.

Issue: #1473
#cherrypick
PiperOrigin-RevId: 646086833
  • Loading branch information
tonihei authored and copybara-github committed Jun 24, 2024
1 parent bb568b5 commit 71ef848
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
*/
package androidx.media3.exoplayer;

import static androidx.media3.common.util.Assertions.checkState;
import static androidx.media3.exoplayer.MediaPeriodQueue.areDurationsCompatible;
import static java.lang.Math.max;

import androidx.annotation.Nullable;
import androidx.media3.common.C;
import androidx.media3.common.Format;
import androidx.media3.common.Timeline;
import androidx.media3.common.util.Assertions;
import androidx.media3.common.util.Log;
import androidx.media3.common.util.NullableType;
import androidx.media3.exoplayer.source.ClippingMediaPeriod;
Expand Down Expand Up @@ -212,7 +212,7 @@ public void handlePrepared(float playbackSpeed, Timeline timeline) throws ExoPla
* @param rendererPositionUs The playing position in renderer time, in microseconds.
*/
public void reevaluateBuffer(long rendererPositionUs) {
Assertions.checkState(isLoadingMediaPeriod());
checkState(isLoadingMediaPeriod());
if (prepared) {
mediaPeriod.reevaluateBuffer(toPeriodTime(rendererPositionUs));
}
Expand All @@ -230,7 +230,7 @@ public void reevaluateBuffer(long rendererPositionUs) {
*/
public void continueLoading(
long rendererPositionUs, float playbackSpeed, long lastRebufferRealtimeMs) {
Assertions.checkState(isLoadingMediaPeriod());
checkState(isLoadingMediaPeriod());
long loadingPeriodPositionUs = toPeriodTime(rendererPositionUs);
mediaPeriod.continueLoading(
new LoadingInfo.Builder()
Expand All @@ -255,6 +255,15 @@ public TrackSelectorResult selectTracks(float playbackSpeed, Timeline timeline)
throws ExoPlaybackException {
TrackSelectorResult selectorResult =
trackSelector.selectTracks(rendererCapabilities, getTrackGroups(), info.id, timeline);
for (int i = 0; i < selectorResult.length; i++) {
if (selectorResult.isRendererEnabled(i)) {
checkState(
selectorResult.selections[i] != null
|| rendererCapabilities[i].getTrackType() == C.TRACK_TYPE_NONE);
} else {
checkState(selectorResult.selections[i] == null);
}
}
for (ExoTrackSelection trackSelection : selectorResult.selections) {
if (trackSelection != null) {
trackSelection.onPlaybackSpeed(playbackSpeed);
Expand Down Expand Up @@ -324,13 +333,13 @@ public long applyTrackSelection(
hasEnabledTracks = false;
for (int i = 0; i < sampleStreams.length; i++) {
if (sampleStreams[i] != null) {
Assertions.checkState(newTrackSelectorResult.isRendererEnabled(i));
checkState(newTrackSelectorResult.isRendererEnabled(i));
// hasEnabledTracks should be true only when non-empty streams exists.
if (rendererCapabilities[i].getTrackType() != C.TRACK_TYPE_NONE) {
hasEnabledTracks = true;
}
} else {
Assertions.checkState(newTrackSelectorResult.selections[i] == null);
checkState(newTrackSelectorResult.selections[i] == null);
}
}
return positionUs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ public final TrackSelectorResult selectTracks(
* @param timeline The {@link Timeline} holding the period for which tracks are to be selected.
* @return A pair consisting of the track selections and configurations for each renderer. A null
* configuration indicates the renderer should be disabled, in which case the track selection
* will also be null. A track selection may also be null for a non-disabled renderer if {@link
* must also be null. A track selection may also be null for a non-disabled renderer if {@link
* RendererCapabilities#getTrackType()} is {@link C#TRACK_TYPE_NONE}.
* @throws ExoPlaybackException If an error occurs while selecting the tracks.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
*/
package androidx.media3.exoplayer.trackselection;

import static androidx.media3.common.util.Assertions.checkArgument;

import androidx.annotation.Nullable;
import androidx.media3.common.C;
import androidx.media3.common.Tracks;
import androidx.media3.common.util.NullableType;
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;
import androidx.media3.exoplayer.RendererCapabilities;
import androidx.media3.exoplayer.RendererConfiguration;

/** The result of a {@link TrackSelector} operation. */
Expand Down Expand Up @@ -69,6 +73,9 @@ public TrackSelectorResult(
* @param rendererConfigurations A {@link RendererConfiguration} for each renderer. A null entry
* indicates the corresponding renderer should be disabled.
* @param selections A {@link ExoTrackSelection} array containing the selection for each renderer.
* If the renderer is disabled with a null {@link RendererConfiguration}, then it must have a
* null selection too. It can also have a null selection if it has a {@linkplain
* RendererCapabilities#getTrackType() track type} of {@link C#TRACK_TYPE_NONE}.
* @param tracks Description of the available tracks and which one were selected.
* @param info An opaque object that will be returned to {@link
* TrackSelector#onSelectionActivated(Object)} should the selection be activated. May be
Expand All @@ -79,6 +86,7 @@ public TrackSelectorResult(
@NullableType ExoTrackSelection[] selections,
Tracks tracks,
@Nullable Object info) {
checkArgument(rendererConfigurations.length == selections.length);
this.rendererConfigurations = rendererConfigurations;
this.selections = selections.clone();
this.tracks = tracks;
Expand Down

0 comments on commit 71ef848

Please sign in to comment.