Fix NPE in ExtractorMediaPeriod

Also turn on nullity checks for ExtractorMediaPeriod. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=208467644

Fix NPE in ExtractorMediaPeriod
Also turn on nullity checks for ExtractorMediaPeriod. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=208467644
32035e1b · andrewlewis · Oliver Woodman · 1c09af02 · 32035e1b · 32035e1b
Commit 32035e1b authored Aug 13, 2018 by andrewlewis Committed by Oliver Woodman Aug 14, 2018
Showing with 111 additions and 44 deletions
library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java
library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java
library/core/src/main/java/com/google/android/exoplayer2/util/Util.java
--- a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java
@@ -49,6 +49,7 @@ import com.google.android.exoplayer2.util.Util;
 import java.io.EOFException;
 import java.io.IOException;
 import java.util.Arrays;
+import org.checkerframework.checker.nullness.compatqual.NullableType;
 /**
 * A {@link MediaPeriod} that extracts data using an {@link Extractor}.
@@ -84,7 +85,7 @@ import java.util.Arrays;
  private final EventDispatcher eventDispatcher;
  private final Listener listener;
  private final Allocator allocator;
-  @Nullable private final String customCacheKey;
+  private final @Nullable String customCacheKey;
  private final long continueLoadingCheckIntervalBytes;
  private final Loader loader;
  private final ExtractorHolder extractorHolder;
@@ -94,23 +95,21 @@ import java.util.Arrays;
  private final Handler handler;
  private @Nullable Callback callback;
-  private SeekMap seekMap;
+  private @Nullable SeekMap seekMap;
  private SampleQueue[] sampleQueues;
  private int[] sampleQueueTrackIds;
  private boolean sampleQueuesBuilt;
  private boolean prepared;
+  private @Nullable PreparedState preparedState;
+  private boolean haveAudioVideoTracks;
  private int dataType;
  private boolean seenFirstTrackSelection;
  private boolean notifyDiscontinuity;
  private boolean notifiedReadingStarted;
  private int enabledTrackCount;
-  private TrackGroupArray tracks;
  private long durationUs;
-  private boolean[] trackEnabledStates;
-  private boolean[] trackIsAudioVideoFlags;
-  private boolean[] trackFormatNotificationSent;
-  private boolean haveAudioVideoTracks;
  private long length;
  private long lastSeekPositionUs;
@@ -134,6 +133,8 @@ import java.util.Arrays;
   * @param continueLoadingCheckIntervalBytes The number of bytes that should be loaded between each
   *     invocation of {@link Callback#onContinueLoadingRequested(SequenceableLoader)}.
   */
+  // maybeFinishPrepare is not posted to the handler until initialization completes.
+  @SuppressWarnings("nullness:methodref.receiver.bound.invalid")
  public ExtractorMediaPeriod(
      Uri uri,
      DataSource dataSource,
@@ -153,11 +154,15 @@ import java.util.Arrays;
    this.customCacheKey = customCacheKey;
    this.continueLoadingCheckIntervalBytes = continueLoadingCheckIntervalBytes;
    loader = new Loader("Loader:ExtractorMediaPeriod");
-    extractorHolder = new ExtractorHolder(extractors, this);
+    extractorHolder = new ExtractorHolder(extractors);
    loadCondition = new ConditionVariable();
    maybeFinishPrepareRunnable = this::maybeFinishPrepare;
    onContinueLoadingRequestedRunnable =
-        () -> callback.onContinueLoadingRequested(ExtractorMediaPeriod.this);
+        () -> {
+          if (!released) {
+            Assertions.checkNotNull(callback).onContinueLoadingRequested(ExtractorMediaPeriod.this);
+          }
+        };
    handler = new Handler();
    sampleQueueTrackIds = new int[0];
    sampleQueues = new SampleQueue[0];
@@ -176,7 +181,7 @@ import java.util.Arrays;
        sampleQueue.discardToEnd();
      }
    }
-    loader.release(this);
+    loader.release(/* callback= */ this);
    handler.removeCallbacksAndMessages(null);
    callback = null;
    released = true;
@@ -205,13 +210,19 @@ import java.util.Arrays;
  @Override
  public TrackGroupArray getTrackGroups() {
-    return tracks;
+    return getPreparedState().tracks;
  }
  @Override
-  public long selectTracks(TrackSelection[] selections, boolean[] mayRetainStreamFlags,
+  public long selectTracks(
-      SampleStream[] streams, boolean[] streamResetFlags, long positionUs) {
+      TrackSelection[] selections,
-    Assertions.checkState(prepared);
+      boolean[] mayRetainStreamFlags,
+      @NullableType SampleStream[] streams,
+      boolean[] streamResetFlags,
+      long positionUs) {
+    PreparedState preparedState = getPreparedState();
+    TrackGroupArray tracks = preparedState.tracks;
+    boolean[] trackEnabledStates = preparedState.trackEnabledStates;
    int oldEnabledTrackCount = enabledTrackCount;
    // Deselect old tracks.
    for (int i = 0; i < selections.length; i++) {
@@ -280,6 +291,7 @@ import java.util.Arrays;
  @Override
  public void discardBuffer(long positionUs, boolean toKeyframe) {
+    boolean[] trackEnabledStates = getPreparedState().trackEnabledStates;
    int trackCount = sampleQueues.length;
    for (int i = 0; i < trackCount; i++) {
      sampleQueues[i].discardTo(positionUs, toKeyframe, trackEnabledStates[i]);
@@ -325,6 +337,7 @@ import java.util.Arrays;
  @Override
  public long getBufferedPositionUs() {
+    boolean[] trackIsAudioVideoFlags = getPreparedState().trackIsAudioVideoFlags;
    if (loadingFinished) {
      return C.TIME_END_OF_SOURCE;
    } else if (isPendingReset()) {
@@ -350,12 +363,15 @@ import java.util.Arrays;
  @Override
  public long seekToUs(long positionUs) {
+    PreparedState preparedState = getPreparedState();
+    SeekMap seekMap = preparedState.seekMap;
+    boolean[] trackIsAudioVideoFlags = preparedState.trackIsAudioVideoFlags;
    // Treat all seeks into non-seekable media as being to t=0.
    positionUs = seekMap.isSeekable() ? positionUs : 0;
    lastSeekPositionUs = positionUs;
    notifyDiscontinuity = false;
    // If we're not pending a reset, see if we can seek within the buffer.
-    if (!isPendingReset() && seekInsideBufferUs(positionUs)) {
+    if (!isPendingReset() && seekInsideBufferUs(trackIsAudioVideoFlags, positionUs)) {
      return positionUs;
    }
    // We were unable to seek within the buffer, so need to reset.
@@ -374,6 +390,7 @@ import java.util.Arrays;
  @Override
  public long getAdjustedSeekPositionUs(long positionUs, SeekParameters seekParameters) {
+    SeekMap seekMap = getPreparedState().seekMap;
    if (!seekMap.isSeekable()) {
      // Treat all seeks into non-seekable media as being to t=0.
      return 0;
@@ -432,6 +449,9 @@ import java.util.Arrays;
  }
  private void maybeNotifyTrackFormat(int track) {
+    PreparedState preparedState = getPreparedState();
+    boolean[] trackFormatNotificationSent = preparedState.trackFormatNotificationSent;
+    TrackGroupArray tracks = preparedState.tracks;
    if (!trackFormatNotificationSent[track]) {
      Format trackFormat = tracks.get(track).getFormat(0);
      eventDispatcher.downstreamFormatChanged(
@@ -445,6 +465,7 @@ import java.util.Arrays;
  }
  private void maybeStartDeferredRetry(int track) {
+    boolean[] trackIsAudioVideoFlags = getPreparedState().trackIsAudioVideoFlags;
    if (!pendingDeferredRetry
        || !trackIsAudioVideoFlags[track]
        || sampleQueues[track].hasNextSample()) {
@@ -458,7 +479,7 @@ import java.util.Arrays;
    for (SampleQueue sampleQueue : sampleQueues) {
      sampleQueue.reset();
    }
-    callback.onContinueLoadingRequested(this);
+    Assertions.checkNotNull(callback).onContinueLoadingRequested(this);
  }
  private boolean suppressRead() {
@@ -471,6 +492,7 @@ import java.util.Arrays;
  public void onLoadCompleted(ExtractingLoadable loadable, long elapsedRealtimeMs,
      long loadDurationMs) {
    if (durationUs == C.TIME_UNSET) {
+      SeekMap seekMap = Assertions.checkNotNull(this.seekMap);
      long largestQueuedTimestampUs = getLargestQueuedTimestampUs();
      durationUs = largestQueuedTimestampUs == Long.MIN_VALUE ? 0
          : largestQueuedTimestampUs + DEFAULT_LAST_SAMPLE_DURATION_US;
@@ -491,7 +513,7 @@ import java.util.Arrays;
        loadable.dataSource.getBytesRead());
    copyLengthFromLoader(loadable);
    loadingFinished = true;
-    callback.onContinueLoadingRequested(this);
+    Assertions.checkNotNull(callback).onContinueLoadingRequested(this);
  }
  @Override
@@ -516,7 +538,7 @@ import java.util.Arrays;
        sampleQueue.reset();
      }
      if (enabledTrackCount > 0) {
-        callback.onContinueLoadingRequested(this);
+        Assertions.checkNotNull(callback).onContinueLoadingRequested(this);
      }
    }
  }
@@ -575,8 +597,9 @@ import java.util.Arrays;
    trackOutput.setUpstreamFormatChangeListener(this);
    sampleQueueTrackIds = Arrays.copyOf(sampleQueueTrackIds, trackCount + 1);
    sampleQueueTrackIds[trackCount] = id;
-    sampleQueues = Arrays.copyOf(sampleQueues, trackCount + 1);
+    @NullableType SampleQueue[] sampleQueues = Arrays.copyOf(this.sampleQueues, trackCount + 1);
    sampleQueues[trackCount] = trackOutput;
+    this.sampleQueues = Util.castNonNullTypeArray(sampleQueues);
    return trackOutput;
  }
@@ -602,7 +625,8 @@ import java.util.Arrays;
  // Internal methods.
  private void maybeFinishPrepare() {
-    if (released || prepared || seekMap == null || !sampleQueuesBuilt) {
+    SeekMap seekMap = this.seekMap;
+    if (released || prepared || !sampleQueuesBuilt || seekMap == null) {
      return;
    }
    for (SampleQueue sampleQueue : sampleQueues) {
@@ -613,9 +637,7 @@ import java.util.Arrays;
    loadCondition.close();
    int trackCount = sampleQueues.length;
    TrackGroup[] trackArray = new TrackGroup[trackCount];
-    trackIsAudioVideoFlags = new boolean[trackCount];
+    boolean[] trackIsAudioVideoFlags = new boolean[trackCount];
-    trackEnabledStates = new boolean[trackCount];
-    trackFormatNotificationSent = new boolean[trackCount];
    durationUs = seekMap.getDurationUs();
    for (int i = 0; i < trackCount; i++) {
      Format trackFormat = sampleQueues[i].getUpstreamFormat();
@@ -625,14 +647,24 @@ import java.util.Arrays;
      trackIsAudioVideoFlags[i] = isAudioVideo;
      haveAudioVideoTracks |= isAudioVideo;
    }
-    tracks = new TrackGroupArray(trackArray);
    dataType =
        length == C.LENGTH_UNSET && seekMap.getDurationUs() == C.TIME_UNSET
            ? C.DATA_TYPE_MEDIA_PROGRESSIVE_LIVE
            : C.DATA_TYPE_MEDIA;
+    preparedState =
+        new PreparedState(
+            new TrackGroupArray(trackArray),
+            /* trackEnabledStates= */ new boolean[trackCount],
+            trackIsAudioVideoFlags,
+            /* trackFormatNotificationSent= */ new boolean[trackCount],
+            seekMap);
    prepared = true;
    listener.onSourceInfoRefreshed(durationUs, seekMap.isSeekable());
-    callback.onPrepared(this);
+    Assertions.checkNotNull(callback).onPrepared(this);
+  }
+  private PreparedState getPreparedState() {
+    return Assertions.checkNotNull(preparedState);
  }
  private void copyLengthFromLoader(ExtractingLoadable loadable) {
@@ -643,8 +675,10 @@ import java.util.Arrays;
  private void startLoading() {
    ExtractingLoadable loadable =
-        new ExtractingLoadable(uri, dataSource, extractorHolder, loadCondition);
+        new ExtractingLoadable(
+            uri, dataSource, extractorHolder, /* extractorOutput= */ this, loadCondition);
    if (prepared) {
+      SeekMap seekMap = getPreparedState().seekMap;
      Assertions.checkState(isPendingReset());
      if (durationUs != C.TIME_UNSET && pendingResetPositionUs >= durationUs) {
        loadingFinished = true;
@@ -719,10 +753,11 @@ import java.util.Arrays;
  /**
   * Attempts to seek to the specified position within the sample queues.
   *
+   * @param trackIsAudioVideoFlags Whether each track is audio/video.
   * @param positionUs The seek position in microseconds.
   * @return Whether the in-buffer seek was successful.
   */
-  private boolean seekInsideBufferUs(long positionUs) {
+  private boolean seekInsideBufferUs(boolean[] trackIsAudioVideoFlags, long positionUs) {
    int trackCount = sampleQueues.length;
    for (int i = 0; i < trackCount; i++) {
      SampleQueue sampleQueue = sampleQueues[i];
@@ -798,6 +833,7 @@ import java.util.Arrays;
    private final Uri uri;
    private final StatsDataSource dataSource;
    private final ExtractorHolder extractorHolder;
+    private final ExtractorOutput extractorOutput;
    private final ConditionVariable loadCondition;
    private final PositionHolder positionHolder;
@@ -812,10 +848,12 @@ import java.util.Arrays;
        Uri uri,
        DataSource dataSource,
        ExtractorHolder extractorHolder,
+        ExtractorOutput extractorOutput,
        ConditionVariable loadCondition) {
-      this.uri = Assertions.checkNotNull(uri);
+      this.uri = uri;
      this.dataSource = new StatsDataSource(dataSource);
-      this.extractorHolder = Assertions.checkNotNull(extractorHolder);
+      this.extractorHolder = extractorHolder;
+      this.extractorOutput = extractorOutput;
      this.loadCondition = loadCondition;
      this.positionHolder = new PositionHolder();
      this.pendingExtractorSeek = true;
@@ -842,8 +880,9 @@ import java.util.Arrays;
          if (length != C.LENGTH_UNSET) {
            length += position;
          }
+          Uri uri = Assertions.checkNotNull(dataSource.getUri());
          input = new DefaultExtractorInput(dataSource, position, length);
-          Extractor extractor = extractorHolder.selectExtractor(input, dataSource.getUri());
+          Extractor extractor = extractorHolder.selectExtractor(input, extractorOutput, uri);
          if (pendingExtractorSeek) {
            extractor.seek(position, seekTimeUs);
            pendingExtractorSeek = false;
@@ -877,24 +916,20 @@ import java.util.Arrays;
    }
  }
-  /**
+  /** Stores a list of extractors and a selected extractor when the format has been detected. */
-   * Stores a list of extractors and a selected extractor when the format has been detected.
-   */
  private static final class ExtractorHolder {
    private final Extractor[] extractors;
-    private final ExtractorOutput extractorOutput;
-    private Extractor extractor;
+    private @Nullable Extractor extractor;
    /**
     * Creates a holder that will select an extractor and initialize it using the specified output.
     *
     * @param extractors One or more extractors to choose from.
-     * @param extractorOutput The output that will be used to initialize the selected extractor.
     */
-    public ExtractorHolder(Extractor[] extractors, ExtractorOutput extractorOutput) {
+    public ExtractorHolder(Extractor[] extractors) {
      this.extractors = extractors;
-      this.extractorOutput = extractorOutput;
    }
    /**
@@ -902,13 +937,15 @@ import java.util.Arrays;
     * later calls.
     *
     * @param input The {@link ExtractorInput} from which data should be read.
+     * @param output The {@link ExtractorOutput} that will be used to initialize the selected
+     *     extractor.
     * @param uri The {@link Uri} of the data.
     * @return An initialized extractor for reading {@code input}.
     * @throws UnrecognizedInputFormatException Thrown if the input format could not be detected.
     * @throws IOException Thrown if the input could not be read.
     * @throws InterruptedException Thrown if the thread was interrupted.
     */
-    public Extractor selectExtractor(ExtractorInput input, Uri uri)
+    public Extractor selectExtractor(ExtractorInput input, ExtractorOutput output, Uri uri)
        throws IOException, InterruptedException {
      if (extractor != null) {
        return extractor;
@@ -929,7 +966,7 @@ import java.util.Arrays;
        throw new UnrecognizedInputFormatException("None of the available extractors ("
            + Util.getCommaDelimitedSimpleClassNames(extractors) + ") could read the stream.", uri);
      }
-      extractor.init(extractorOutput);
+      extractor.init(output);
      return extractor;
    }
@@ -940,4 +977,26 @@ import java.util.Arrays;
      }
    }
  }
+  /** Stores state that is initialized when preparation completes. */
+  private static final class PreparedState {
+    public final TrackGroupArray tracks;
+    public final boolean[] trackEnabledStates;
+    public final boolean[] trackIsAudioVideoFlags;
+    public final boolean[] trackFormatNotificationSent;
+    public final SeekMap seekMap;
+    public PreparedState(
+        TrackGroupArray tracks,
+        boolean[] trackEnabledStates,
+        boolean[] trackIsAudioVideoFlags,
+        boolean[] trackFormatNotificationSent,
+        SeekMap seekMap) {
+      this.tracks = tracks;
+      this.trackEnabledStates = trackEnabledStates;
+      this.trackIsAudioVideoFlags = trackIsAudioVideoFlags;
+      this.trackFormatNotificationSent = trackFormatNotificationSent;
+      this.seekMap = seekMap;
+    }
+  }
 }
--- a/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java
@@ -123,7 +123,7 @@ public final class MimeTypes {
   * @param mimeType The mimeType to test.
   * @return Whether the top level type is audio.
   */
-  public static boolean isAudio(String mimeType) {
+  public static boolean isAudio(@Nullable String mimeType) {
    return BASE_TYPE_AUDIO.equals(getTopLevelType(mimeType));
  }
@@ -133,7 +133,7 @@ public final class MimeTypes {
   * @param mimeType The mimeType to test.
   * @return Whether the top level type is video.
   */
-  public static boolean isVideo(String mimeType) {
+  public static boolean isVideo(@Nullable String mimeType) {
    return BASE_TYPE_VIDEO.equals(getTopLevelType(mimeType));
  }
@@ -143,7 +143,7 @@ public final class MimeTypes {
   * @param mimeType The mimeType to test.
   * @return Whether the top level type is text.
   */
-  public static boolean isText(String mimeType) {
+  public static boolean isText(@Nullable String mimeType) {
    return BASE_TYPE_TEXT.equals(getTopLevelType(mimeType));
  }
@@ -153,7 +153,7 @@ public final class MimeTypes {
   * @param mimeType The mimeType to test.
   * @return Whether the top level type is application.
   */
-  public static boolean isApplication(String mimeType) {
+  public static boolean isApplication(@Nullable String mimeType) {
    return BASE_TYPE_APPLICATION.equals(getTopLevelType(mimeType));
  }

--- a/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java
@@ -73,6 +73,7 @@ import java.util.regex.Pattern;
 import java.util.zip.DataFormatException;
 import java.util.zip.Inflater;
 import org.checkerframework.checker.initialization.qual.UnknownInitialization;
+import org.checkerframework.checker.nullness.compatqual.NullableType;
 import org.checkerframework.checker.nullness.qual.EnsuresNonNull;
 import org.checkerframework.checker.nullness.qual.PolyNull;
@@ -271,6 +272,13 @@ public final class Util {
    return value;
  }
+  /** Casts a nullable type array to a non-null type array without runtime null check. */
+  @SuppressWarnings({"contracts.postcondition.not.satisfied", "return.type.incompatible"})
+  @EnsuresNonNull("#1")
+  public static <T> T[] castNonNullTypeArray(@NullableType T[] value) {
+    return value;
+  }
  /**
   * Copies and optionally truncates an array. Prevents null array elements created by {@link
   * Arrays#copyOf(Object[], int)} by ensuring the new length does not exceed the current length.