Move playback error into PlaybackInfo.

The error is closely related to the playback state IDLE and should be updated in sync with the state to prevent unexpected event ordering and/or keeping the error after re-preparation. Issue:#5407 PiperOrigin-RevId: 265014630
google · Aug 23, 2019 · 29af689 · 29af689
1 parent 7883eab
commit 29af689
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 27 deletions.
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
@@ -74,7 +74,6 @@
   private int pendingSetPlaybackParametersAcks;
   private PlaybackParameters playbackParameters;
   private SeekParameters seekParameters;
-  @Nullable private ExoPlaybackException playbackError;
 
   // Playback information when there is no pending seek/set source operation.
   private PlaybackInfo playbackInfo;
@@ -202,13 +201,12 @@ public int getPlaybackState() {
   @Override
   @Nullable
   public ExoPlaybackException getPlaybackError() {
-    return playbackError;
+    return playbackInfo.playbackError;
   }
 
   @Override
   public void retry() {
-    if (mediaSource != null
-        && (playbackError != null || playbackInfo.playbackState == Player.STATE_IDLE)) {
+    if (mediaSource != null && playbackInfo.playbackState == Player.STATE_IDLE) {
       prepare(mediaSource, /* resetPosition= */ false, /* resetState= */ false);
     }
   }
@@ -220,11 +218,13 @@ public void prepare(MediaSource mediaSource) {
 
   @Override
   public void prepare(MediaSource mediaSource, boolean resetPosition, boolean resetState) {
-    playbackError = null;
     this.mediaSource = mediaSource;
     PlaybackInfo playbackInfo =
         getResetPlaybackInfo(
-            resetPosition, resetState, /* playbackState= */ Player.STATE_BUFFERING);
+            resetPosition,
+            resetState,
+            /* resetError= */ true,
+            /* playbackState= */ Player.STATE_BUFFERING);
     // Trigger internal prepare first before updating the playback info and notifying external
     // listeners to ensure that new operations issued in the listener notifications reach the
     // player after this prepare. The internal player can't change the playback info immediately
@@ -381,13 +381,13 @@ public void setForegroundMode(boolean foregroundMode) {
   @Override
   public void stop(boolean reset) {
     if (reset) {
-      playbackError = null;
       mediaSource = null;
     }
     PlaybackInfo playbackInfo =
         getResetPlaybackInfo(
             /* resetPosition= */ reset,
             /* resetState= */ reset,
+            /* resetError= */ reset,
             /* playbackState= */ Player.STATE_IDLE);
     // Trigger internal stop first before updating the playback info and notifying external
     // listeners to ensure that new operations issued in the listener notifications reach the
@@ -415,6 +415,7 @@ public void release() {
         getResetPlaybackInfo(
             /* resetPosition= */ false,
             /* resetState= */ false,
+            /* resetError= */ false,
             /* playbackState= */ Player.STATE_IDLE);
   }
 
@@ -572,11 +573,6 @@ public Timeline getCurrentTimeline() {
       case ExoPlayerImplInternal.MSG_PLAYBACK_PARAMETERS_CHANGED:
         handlePlaybackParameters((PlaybackParameters) msg.obj, /* operationAck= */ msg.arg1 != 0);
         break;
-      case ExoPlayerImplInternal.MSG_ERROR:
-        ExoPlaybackException playbackError = (ExoPlaybackException) msg.obj;
-        this.playbackError = playbackError;
-        notifyListeners(listener -> listener.onPlayerError(playbackError));
-        break;
       default:
         throw new IllegalStateException();
     }
@@ -635,7 +631,10 @@ private void handlePlaybackInfo(
   }
 
   private PlaybackInfo getResetPlaybackInfo(
-      boolean resetPosition, boolean resetState, @Player.State int playbackState) {
+      boolean resetPosition,
+      boolean resetState,
+      boolean resetError,
+      @Player.State int playbackState) {
     if (resetPosition) {
       maskingWindowIndex = 0;
       maskingPeriodIndex = 0;
@@ -659,6 +658,7 @@ private PlaybackInfo getResetPlaybackInfo(
         startPositionUs,
         contentPositionUs,
         playbackState,
+        resetError ? null : playbackInfo.playbackError,
         /* isLoading= */ false,
         resetState ? TrackGroupArray.EMPTY : playbackInfo.trackGroups,
         resetState ? emptyTrackSelectorResult : playbackInfo.trackSelectorResult,
@@ -728,6 +728,7 @@ private static final class PlaybackInfoUpdate implements Runnable {
     private final @Player.TimelineChangeReason int timelineChangeReason;
     private final boolean seekProcessed;
     private final boolean playbackStateChanged;
+    private final boolean playbackErrorChanged;
     private final boolean timelineChanged;
     private final boolean isLoadingChanged;
     private final boolean trackSelectorResultChanged;
@@ -752,6 +753,9 @@ public PlaybackInfoUpdate(
       this.seekProcessed = seekProcessed;
       this.playWhenReady = playWhenReady;
       playbackStateChanged = previousPlaybackInfo.playbackState != playbackInfo.playbackState;
+      playbackErrorChanged =
+          previousPlaybackInfo.playbackError != playbackInfo.playbackError
+              && playbackInfo.playbackError != null;
       timelineChanged = previousPlaybackInfo.timeline != playbackInfo.timeline;
       isLoadingChanged = previousPlaybackInfo.isLoading != playbackInfo.isLoading;
       trackSelectorResultChanged =
@@ -770,6 +774,9 @@ public void run() {
             listenerSnapshot,
             listener -> listener.onPositionDiscontinuity(positionDiscontinuityReason));
       }
+      if (playbackErrorChanged) {
+        invokeAll(listenerSnapshot, listener -> listener.onPlayerError(playbackInfo.playbackError));
+      }
       if (trackSelectorResultChanged) {
         trackSelector.onSelectionActivated(playbackInfo.trackSelectorResult.info);
         invokeAll(

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
@@ -61,7 +61,6 @@
   // External messages
   public static final int MSG_PLAYBACK_INFO_CHANGED = 0;
   public static final int MSG_PLAYBACK_PARAMETERS_CHANGED = 1;
-  public static final int MSG_ERROR = 2;
 
   // Internal messages
   private static final int MSG_PREPARE = 0;
@@ -374,31 +373,31 @@ public boolean handleMessage(Message msg) {
       maybeNotifyPlaybackInfoChanged();
     } catch (ExoPlaybackException e) {
       Log.e(TAG, "Playback error.", e);
-      eventHandler.obtainMessage(MSG_ERROR, e).sendToTarget();
       stopInternal(
           /* forceResetRenderers= */ true,
           /* resetPositionAndState= */ false,
           /* acknowledgeStop= */ false);
+      playbackInfo = playbackInfo.copyWithPlaybackError(e);
       maybeNotifyPlaybackInfoChanged();
     } catch (IOException e) {
       Log.e(TAG, "Source error.", e);
-      eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForSource(e)).sendToTarget();
       stopInternal(
           /* forceResetRenderers= */ false,
           /* resetPositionAndState= */ false,
           /* acknowledgeStop= */ false);
+      playbackInfo = playbackInfo.copyWithPlaybackError(ExoPlaybackException.createForSource(e));
       maybeNotifyPlaybackInfoChanged();
     } catch (RuntimeException | OutOfMemoryError e) {
       Log.e(TAG, "Internal runtime error.", e);
       ExoPlaybackException error =
           e instanceof OutOfMemoryError
               ? ExoPlaybackException.createForOutOfMemoryError((OutOfMemoryError) e)
               : ExoPlaybackException.createForUnexpected((RuntimeException) e);
-      eventHandler.obtainMessage(MSG_ERROR, error).sendToTarget();
       stopInternal(
           /* forceResetRenderers= */ true,
           /* resetPositionAndState= */ false,
           /* acknowledgeStop= */ false);
+      playbackInfo = playbackInfo.copyWithPlaybackError(error);
       maybeNotifyPlaybackInfoChanged();
     }
     return true;
@@ -436,7 +435,11 @@ private void maybeNotifyPlaybackInfoChanged() {
   private void prepareInternal(MediaSource mediaSource, boolean resetPosition, boolean resetState) {
     pendingPrepareCount++;
     resetInternal(
-        /* resetRenderers= */ false, /* releaseMediaSource= */ true, resetPosition, resetState);
+        /* resetRenderers= */ false,
+        /* releaseMediaSource= */ true,
+        resetPosition,
+        resetState,
+        /* resetError= */ true);
     loadControl.onPrepared();
     this.mediaSource = mediaSource;
     setState(Player.STATE_BUFFERING);
@@ -688,7 +691,8 @@ private void seekToInternal(SeekPosition seekPosition) throws ExoPlaybackExcepti
             /* resetRenderers= */ false,
             /* releaseMediaSource= */ false,
             /* resetPosition= */ true,
-            /* resetState= */ false);
+            /* resetState= */ false,
+            /* resetError= */ true);
       } else {
         // Execute the seek in the current media periods.
         long newPeriodPositionUs = periodPositionUs;
@@ -834,7 +838,8 @@ private void stopInternal(
         /* resetRenderers= */ forceResetRenderers || !foregroundMode,
         /* releaseMediaSource= */ true,
         /* resetPosition= */ resetPositionAndState,
-        /* resetState= */ resetPositionAndState);
+        /* resetState= */ resetPositionAndState,
+        /* resetError= */ resetPositionAndState);
     playbackInfoUpdate.incrementPendingOperationAcks(
         pendingPrepareCount + (acknowledgeStop ? 1 : 0));
     pendingPrepareCount = 0;
@@ -847,7 +852,8 @@ private void releaseInternal() {
         /* resetRenderers= */ true,
         /* releaseMediaSource= */ true,
         /* resetPosition= */ true,
-        /* resetState= */ true);
+        /* resetState= */ true,
+        /* resetError= */ false);
     loadControl.onReleased();
     setState(Player.STATE_IDLE);
     internalPlaybackThread.quit();
@@ -861,7 +867,8 @@ private void resetInternal(
       boolean resetRenderers,
       boolean releaseMediaSource,
       boolean resetPosition,
-      boolean resetState) {
+      boolean resetState,
+      boolean resetError) {
     handler.removeMessages(MSG_DO_SOME_WORK);
     rebuffering = false;
     mediaClock.stop();
@@ -924,6 +931,7 @@ private void resetInternal(
             startPositionUs,
             contentPositionUs,
             playbackInfo.playbackState,
+            resetError ? null : playbackInfo.playbackError,
             /* isLoading= */ false,
             resetState ? TrackGroupArray.EMPTY : playbackInfo.trackGroups,
             resetState ? emptyTrackSelectorResult : playbackInfo.trackSelectorResult,
@@ -1382,7 +1390,8 @@ private void handleSourceInfoRefreshEndedPlayback() {
         /* resetRenderers= */ false,
         /* releaseMediaSource= */ false,
         /* resetPosition= */ true,
-        /* resetState= */ false);
+        /* resetState= */ false,
+        /* resetError= */ true);
   }
 
   /**

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java
@@ -16,6 +16,7 @@
 package com.google.android.exoplayer2;
 
 import androidx.annotation.CheckResult;
+import androidx.annotation.Nullable;
 import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId;
 import com.google.android.exoplayer2.source.TrackGroupArray;
 import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
@@ -51,6 +52,8 @@
   public final long contentPositionUs;
   /** The current playback state. One of the {@link Player}.STATE_ constants. */
   @Player.State public final int playbackState;
+  /** The current playback error, or null if this is not an error state. */
+  @Nullable public final ExoPlaybackException playbackError;
   /** Whether the player is currently loading. */
   public final boolean isLoading;
   /** The currently available track groups. */
@@ -93,6 +96,7 @@ public static PlaybackInfo createDummy(
         startPositionUs,
         /* contentPositionUs= */ C.TIME_UNSET,
         Player.STATE_IDLE,
+        /* playbackError= */ null,
         /* isLoading= */ false,
         TrackGroupArray.EMPTY,
         emptyTrackSelectorResult,
@@ -124,6 +128,7 @@ public PlaybackInfo(
       long startPositionUs,
       long contentPositionUs,
       @Player.State int playbackState,
+      @Nullable ExoPlaybackException playbackError,
       boolean isLoading,
       TrackGroupArray trackGroups,
       TrackSelectorResult trackSelectorResult,
@@ -136,6 +141,7 @@ public PlaybackInfo(
     this.startPositionUs = startPositionUs;
     this.contentPositionUs = contentPositionUs;
     this.playbackState = playbackState;
+    this.playbackError = playbackError;
     this.isLoading = isLoading;
     this.trackGroups = trackGroups;
     this.trackSelectorResult = trackSelectorResult;
@@ -194,6 +200,7 @@ public PlaybackInfo copyWithNewPosition(
         positionUs,
         periodId.isAd() ? contentPositionUs : C.TIME_UNSET,
         playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,
@@ -217,6 +224,7 @@ public PlaybackInfo copyWithTimeline(Timeline timeline) {
         startPositionUs,
         contentPositionUs,
         playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,
@@ -240,6 +248,31 @@ public PlaybackInfo copyWithPlaybackState(int playbackState) {
         startPositionUs,
         contentPositionUs,
         playbackState,
+        playbackError,
+        isLoading,
+        trackGroups,
+        trackSelectorResult,
+        loadingMediaPeriodId,
+        bufferedPositionUs,
+        totalBufferedDurationUs,
+        positionUs);
+  }
+
+  /**
+   * Copies playback info with a playback error.
+   *
+   * @param playbackError The error. See {@link #playbackError}.
+   * @return Copied playback info with the playback error.
+   */
+  @CheckResult
+  public PlaybackInfo copyWithPlaybackError(@Nullable ExoPlaybackException playbackError) {
+    return new PlaybackInfo(
+        timeline,
+        periodId,
+        startPositionUs,
+        contentPositionUs,
+        playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,
@@ -263,6 +296,7 @@ public PlaybackInfo copyWithIsLoading(boolean isLoading) {
         startPositionUs,
         contentPositionUs,
         playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,
@@ -288,6 +322,7 @@ public PlaybackInfo copyWithTrackInfo(
         startPositionUs,
         contentPositionUs,
         playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,
@@ -311,6 +346,7 @@ public PlaybackInfo copyWithLoadingMediaPeriodId(MediaPeriodId loadingMediaPerio
         startPositionUs,
         contentPositionUs,
         playbackState,
+        playbackError,
         isLoading,
         trackGroups,
         trackSelectorResult,

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java
@@ -465,10 +465,7 @@ public final void onShuffleModeEnabledChanged(boolean shuffleModeEnabled) {
 
   @Override
   public final void onPlayerError(ExoPlaybackException error) {
-    EventTime eventTime =
-        error.type == ExoPlaybackException.TYPE_SOURCE
-            ? generateLoadingMediaPeriodEventTime()
-            : generatePlayingMediaPeriodEventTime();
+    EventTime eventTime = generateLastReportedPlayingMediaPeriodEventTime();
     for (AnalyticsListener listener : listeners) {
       listener.onPlayerError(eventTime, error);
     }

diff --git a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java
@@ -359,6 +359,7 @@ private void setupTimeline(long initialPositionUs, long... adGroupTimesUs) {
             /* startPositionUs= */ 0,
             /* contentPositionUs= */ 0,
             Player.STATE_READY,
+            /* playbackError= */ null,
             /* isLoading= */ false,
             /* trackGroups= */ null,
             /* trackSelectorResult= */ null,