Skip to content

Commit

Permalink
Add error messages to correctness assertions in SimpleBasePlayer
Browse files Browse the repository at this point in the history 
Users of this class may run into these assertions when creating the
State and they need to check the source code to understand why
the State is invalid. Adding error messages to all our correctness
assertions helps to understand the root cause more easily.

PiperOrigin-RevId: 496875109
  • Loading branch information
@tonihei @tianyif
tonihei authored and tianyif committed Dec 21, 2022
1 parent a4c3038 commit 6c98f23
Showing 1 changed file with 64 additions and 36 deletions.
100 changes: 64 additions & 36 deletions libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ public Builder setTimedMetadata(Metadata timedMetadata) {
public Builder setPlaylist(List<MediaItemData> playlist) {
HashSet<Object> uids = new HashSet<>();
for (int i = 0; i < playlist.size(); i++) {
checkArgument(uids.add(playlist.get(i).uid));
checkArgument(uids.add(playlist.get(i).uid), "Duplicate MediaItemData UID in playlist");
}
this.playlist = ImmutableList.copyOf(playlist);
this.timeline = new PlaylistTimeline(this.playlist);
Expand Down Expand Up @@ -882,16 +882,20 @@ private State(Builder builder) {
if (builder.timeline.isEmpty()) {
checkArgument(
builder.playbackState == Player.STATE_IDLE
|| builder.playbackState == Player.STATE_ENDED);
|| builder.playbackState == Player.STATE_ENDED,
"Empty playlist only allowed in STATE_IDLE or STATE_ENDED");
checkArgument(
builder.currentAdGroupIndex == C.INDEX_UNSET
&& builder.currentAdIndexInAdGroup == C.INDEX_UNSET);
&& builder.currentAdIndexInAdGroup == C.INDEX_UNSET,
"Ads not allowed if playlist is empty");
} else {
int mediaItemIndex = builder.currentMediaItemIndex;
if (mediaItemIndex == C.INDEX_UNSET) {
mediaItemIndex = 0; // TODO: Use shuffle order to find first index.
} else {
checkArgument(builder.currentMediaItemIndex < builder.timeline.getWindowCount());
checkArgument(
builder.currentMediaItemIndex < builder.timeline.getWindowCount(),
"currentMediaItemIndex must be less than playlist.size()");
}
if (builder.currentAdGroupIndex != C.INDEX_UNSET) {
Timeline.Period period = new Timeline.Period();
Expand All @@ -904,19 +908,25 @@ private State(Builder builder) {
getPeriodIndexFromWindowPosition(
builder.timeline, mediaItemIndex, contentPositionMs, window, period);
builder.timeline.getPeriod(periodIndex, period);
checkArgument(builder.currentAdGroupIndex < period.getAdGroupCount());
checkArgument(
builder.currentAdGroupIndex < period.getAdGroupCount(),
"PeriodData has less ad groups than adGroupIndex");
int adCountInGroup = period.getAdCountInAdGroup(builder.currentAdGroupIndex);
if (adCountInGroup != C.LENGTH_UNSET) {
checkArgument(builder.currentAdIndexInAdGroup < adCountInGroup);
checkArgument(
builder.currentAdIndexInAdGroup < adCountInGroup,
"Ad group has less ads than adIndexInGroupIndex");
}
}
}
if (builder.playerError != null) {
checkArgument(builder.playbackState == Player.STATE_IDLE);
checkArgument(
builder.playbackState == Player.STATE_IDLE, "Player error only allowed in STATE_IDLE");
}
if (builder.playbackState == Player.STATE_IDLE
|| builder.playbackState == Player.STATE_ENDED) {
checkArgument(!builder.isLoading);
checkArgument(
!builder.isLoading, "isLoading only allowed when not in STATE_IDLE or STATE_ENDED");
}
PositionSupplier contentPositionMsSupplier = builder.contentPositionMsSupplier;
if (builder.contentPositionMs != null) {
Expand Down Expand Up @@ -1494,9 +1504,12 @@ public Builder setIsPlaceholder(boolean isPlaceholder) {
public Builder setPeriods(List<PeriodData> periods) {
int periodCount = periods.size();
for (int i = 0; i < periodCount - 1; i++) {
checkArgument(periods.get(i).durationUs != C.TIME_UNSET);
checkArgument(
periods.get(i).durationUs != C.TIME_UNSET, "Periods other than last need a duration");
for (int j = i + 1; j < periodCount; j++) {
checkArgument(!periods.get(i).uid.equals(periods.get(j).uid));
checkArgument(
!periods.get(i).uid.equals(periods.get(j).uid),
"Duplicate PeriodData UIDs in period list");
}
}
this.periods = ImmutableList.copyOf(periods);
Expand Down Expand Up @@ -1575,16 +1588,26 @@ public MediaItemData build() {

private MediaItemData(Builder builder) {
if (builder.liveConfiguration == null) {
checkArgument(builder.presentationStartTimeMs == C.TIME_UNSET);
checkArgument(builder.windowStartTimeMs == C.TIME_UNSET);
checkArgument(builder.elapsedRealtimeEpochOffsetMs == C.TIME_UNSET);
checkArgument(
builder.presentationStartTimeMs == C.TIME_UNSET,
"presentationStartTimeMs can only be set if liveConfiguration != null");
checkArgument(
builder.windowStartTimeMs == C.TIME_UNSET,
"windowStartTimeMs can only be set if liveConfiguration != null");
checkArgument(
builder.elapsedRealtimeEpochOffsetMs == C.TIME_UNSET,
"elapsedRealtimeEpochOffsetMs can only be set if liveConfiguration != null");
} else if (builder.presentationStartTimeMs != C.TIME_UNSET
&& builder.windowStartTimeMs != C.TIME_UNSET) {
checkArgument(builder.windowStartTimeMs >= builder.presentationStartTimeMs);
checkArgument(
builder.windowStartTimeMs >= builder.presentationStartTimeMs,
"windowStartTimeMs can't be less than presentationStartTimeMs");
}
int periodCount = builder.periods.size();
if (builder.durationUs != C.TIME_UNSET) {
checkArgument(builder.defaultPositionUs <= builder.durationUs);
checkArgument(
builder.defaultPositionUs <= builder.durationUs,
"defaultPositionUs can't be greater than durationUs");
}
this.uid = builder.uid;
this.tracks = builder.tracks;
Expand Down Expand Up @@ -2782,7 +2805,7 @@ protected MediaItemData getPlaceholderMediaItemData(MediaItem mediaItem) {
*/
@ForOverride
protected ListenableFuture<?> handleSetPlayWhenReady(boolean playWhenReady) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_PLAY_PAUSE");
}

/**
Expand All @@ -2795,7 +2818,7 @@ protected ListenableFuture<?> handleSetPlayWhenReady(boolean playWhenReady) {
*/
@ForOverride
protected ListenableFuture<?> handlePrepare() {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_PREPARE");
}

/**
Expand All @@ -2808,7 +2831,7 @@ protected ListenableFuture<?> handlePrepare() {
*/
@ForOverride
protected ListenableFuture<?> handleStop() {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_STOP");
}

/**
Expand All @@ -2820,7 +2843,7 @@ protected ListenableFuture<?> handleStop() {
// TODO(b/261158047): Add that this method will only be called if COMMAND_RELEASE is available.
@ForOverride
protected ListenableFuture<?> handleRelease() {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_RELEASE");
}

/**
Expand All @@ -2834,7 +2857,7 @@ protected ListenableFuture<?> handleRelease() {
*/
@ForOverride
protected ListenableFuture<?> handleSetRepeatMode(@RepeatMode int repeatMode) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_REPEAT_MODE");
}

/**
Expand All @@ -2848,7 +2871,7 @@ protected ListenableFuture<?> handleSetRepeatMode(@RepeatMode int repeatMode) {
*/
@ForOverride
protected ListenableFuture<?> handleSetShuffleModeEnabled(boolean shuffleModeEnabled) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_SHUFFLE_MODE");
}

/**
Expand All @@ -2862,7 +2885,7 @@ protected ListenableFuture<?> handleSetShuffleModeEnabled(boolean shuffleModeEna
*/
@ForOverride
protected ListenableFuture<?> handleSetPlaybackParameters(PlaybackParameters playbackParameters) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_SPEED_AND_PITCH");
}

/**
Expand All @@ -2877,7 +2900,8 @@ protected ListenableFuture<?> handleSetPlaybackParameters(PlaybackParameters pla
@ForOverride
protected ListenableFuture<?> handleSetTrackSelectionParameters(
TrackSelectionParameters trackSelectionParameters) {
throw new IllegalStateException();
throw new IllegalStateException(
"Missing implementation to handle COMMAND_SET_TRACK_SELECTION_PARAMETERS");
}

/**
Expand All @@ -2891,7 +2915,8 @@ protected ListenableFuture<?> handleSetTrackSelectionParameters(
*/
@ForOverride
protected ListenableFuture<?> handleSetPlaylistMetadata(MediaMetadata playlistMetadata) {
throw new IllegalStateException();
throw new IllegalStateException(
"Missing implementation to handle COMMAND_SET_MEDIA_ITEMS_METADATA");
}

/**
Expand All @@ -2906,7 +2931,7 @@ protected ListenableFuture<?> handleSetPlaylistMetadata(MediaMetadata playlistMe
*/
@ForOverride
protected ListenableFuture<?> handleSetVolume(@FloatRange(from = 0, to = 1.0) float volume) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_VOLUME");
}

/**
Expand All @@ -2920,7 +2945,7 @@ protected ListenableFuture<?> handleSetVolume(@FloatRange(from = 0, to = 1.0) fl
*/
@ForOverride
protected ListenableFuture<?> handleSetDeviceVolume(@IntRange(from = 0) int deviceVolume) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_DEVICE_VOLUME");
}

/**
Expand All @@ -2933,7 +2958,8 @@ protected ListenableFuture<?> handleSetDeviceVolume(@IntRange(from = 0) int devi
*/
@ForOverride
protected ListenableFuture<?> handleIncreaseDeviceVolume() {
throw new IllegalStateException();
throw new IllegalStateException(
"Missing implementation to handle COMMAND_ADJUST_DEVICE_VOLUME");
}

/**
Expand All @@ -2946,7 +2972,8 @@ protected ListenableFuture<?> handleIncreaseDeviceVolume() {
*/
@ForOverride
protected ListenableFuture<?> handleDecreaseDeviceVolume() {
throw new IllegalStateException();
throw new IllegalStateException(
"Missing implementation to handle COMMAND_ADJUST_DEVICE_VOLUME");
}

/**
Expand All @@ -2960,7 +2987,8 @@ protected ListenableFuture<?> handleDecreaseDeviceVolume() {
*/
@ForOverride
protected ListenableFuture<?> handleSetDeviceMuted(boolean muted) {
throw new IllegalStateException();
throw new IllegalStateException(
"Missing implementation to handle COMMAND_ADJUST_DEVICE_VOLUME");
}

/**
Expand All @@ -2975,7 +3003,7 @@ protected ListenableFuture<?> handleSetDeviceMuted(boolean muted) {
*/
@ForOverride
protected ListenableFuture<?> handleSetVideoOutput(Object videoOutput) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_VIDEO_SURFACE");
}

/**
Expand All @@ -2992,7 +3020,7 @@ protected ListenableFuture<?> handleSetVideoOutput(Object videoOutput) {
*/
@ForOverride
protected ListenableFuture<?> handleClearVideoOutput(@Nullable Object videoOutput) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_VIDEO_SURFACE");
}

/**
Expand All @@ -3013,7 +3041,7 @@ protected ListenableFuture<?> handleClearVideoOutput(@Nullable Object videoOutpu
@ForOverride
protected ListenableFuture<?> handleSetMediaItems(
List<MediaItem> mediaItems, int startIndex, long startPositionMs) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_SET_MEDIA_ITEM(S)");
}

/**
Expand All @@ -3029,7 +3057,7 @@ protected ListenableFuture<?> handleSetMediaItems(
*/
@ForOverride
protected ListenableFuture<?> handleAddMediaItems(int index, List<MediaItem> mediaItems) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_CHANGE_MEDIA_ITEMS");
}

/**
Expand All @@ -3049,7 +3077,7 @@ protected ListenableFuture<?> handleAddMediaItems(int index, List<MediaItem> med
*/
@ForOverride
protected ListenableFuture<?> handleMoveMediaItems(int fromIndex, int toIndex, int newIndex) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_CHANGE_MEDIA_ITEMS");
}

/**
Expand All @@ -3066,7 +3094,7 @@ protected ListenableFuture<?> handleMoveMediaItems(int fromIndex, int toIndex, i
*/
@ForOverride
protected ListenableFuture<?> handleRemoveMediaItems(int fromIndex, int toIndex) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle COMMAND_CHANGE_MEDIA_ITEMS");
}

/**
Expand All @@ -3087,7 +3115,7 @@ protected ListenableFuture<?> handleRemoveMediaItems(int fromIndex, int toIndex)
@ForOverride
protected ListenableFuture<?> handleSeek(
int mediaItemIndex, long positionMs, @Player.Command int seekCommand) {
throw new IllegalStateException();
throw new IllegalStateException("Missing implementation to handle one of the COMMAND_SEEK_*");
}

@RequiresNonNull("state")
Expand Down

0 comments on commit 6c98f23

Please sign in to comment.