Final fixes for correctly handling chunk load failures in HLS.

An accumulation of several fixes:

1. Change to HlsExtractorWrapper is just a move + documentating
   things that were already true + adding a precondition in the
   configureSpliceTo method.

2. Change in HlsSampleSource.readData ensures that configureSpliceTo
   and hasSamples aren't called on an extractor that isn't prepared.

3. The other change in HlsSampleSource ensures the correct "previous"
   TsChunk is used. If a TsChunk fails to load and is replaced, the
   previous chunk should be the one before that whose load completed
   successfully.

4. Determine switchingVariantSpliced based on the actual format of the
   previous chunk, so it's set correctly in the case of a TsChunk load
   failure and subsequent replacement.

library/src/main/java/com/google/android/exoplayer/hls/HlsChunkSource.java

@@ -264,7 +264,8 @@ public class HlsChunkSource {
       switchingVariantSpliced = false;
       switchingVariantSpliced = false;
     } else {
     } else {
       nextVariantIndex = getNextVariantIndex(previousTsChunk, playbackPositionUs);
       nextVariantIndex = getNextVariantIndex(previousTsChunk, playbackPositionUs);
-      switchingVariantSpliced = nextVariantIndex != selectedVariantIndex
+      switchingVariantSpliced = previousTsChunk != null
+          && !variants[nextVariantIndex].format.equals(previousTsChunk.format)
           && adaptiveMode == ADAPTIVE_MODE_SPLICE;
           && adaptiveMode == ADAPTIVE_MODE_SPLICE;
     }
     }
 
 

library/src/main/java/com/google/android/exoplayer/hls/HlsExtractorWrapper.java

@@ -74,6 +74,46 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
   }
   }
 
 
   /**
   /**
+   * Whether the extractor is prepared.
+   *
+   * @return True if the extractor is prepared. False otherwise.
+   */
+  public boolean isPrepared() {
+    if (!prepared && tracksBuilt) {
+      for (int i = 0; i < sampleQueues.size(); i++) {
+        if (!sampleQueues.valueAt(i).hasFormat()) {
+          return false;
+        }
+      }
+      prepared = true;
+    }
+    return prepared;
+  }
+
+  /**
+   * Clears queues for all tracks, returning all allocations to the allocator.
+   */
+  public void clear() {
+    for (int i = 0; i < sampleQueues.size(); i++) {
+      sampleQueues.valueAt(i).clear();
+    }
+  }
+
+  /**
+   * Gets the largest timestamp of any sample parsed by the extractor.
+   *
+   * @return The largest timestamp, or {@link Long#MIN_VALUE} if no samples have been parsed.
+   */
+  public long getLargestParsedTimestampUs() {
+    long largestParsedTimestampUs = Long.MIN_VALUE;
+    for (int i = 0; i < sampleQueues.size(); i++) {
+      largestParsedTimestampUs = Math.max(largestParsedTimestampUs,
+          sampleQueues.valueAt(i).getLargestParsedTimestampUs());
+    }
+    return largestParsedTimestampUs;
+  }
+
+  /**
    * Attempts to configure a splice from this extractor to the next.
    * Attempts to configure a splice from this extractor to the next.
    * <p>
    * <p>
    * The splice is performed such that for each track the samples read from the next extractor
    * The splice is performed such that for each track the samples read from the next extractor
@@ -84,10 +124,13 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
    * splice to be performed. Calling this method is a noop if the splice has already been
    * splice to be performed. Calling this method is a noop if the splice has already been
    * configured. Hence this method should be called repeatedly during the window within which a
    * configured. Hence this method should be called repeatedly during the window within which a
    * splice can be performed.
    * splice can be performed.
+   * <p>
+   * This method must only be called after the extractor has been prepared.
    *
    *
    * @param nextExtractor The extractor being spliced to.
    * @param nextExtractor The extractor being spliced to.
    */
    */
   public final void configureSpliceTo(HlsExtractorWrapper nextExtractor) {
   public final void configureSpliceTo(HlsExtractorWrapper nextExtractor) {
+    Assertions.checkState(isPrepared());
     if (spliceConfigured || !nextExtractor.shouldSpliceIn || !nextExtractor.isPrepared()) {
     if (spliceConfigured || !nextExtractor.shouldSpliceIn || !nextExtractor.isPrepared()) {
       // The splice is already configured, or the next extractor doesn't want to be spliced in, or
       // The splice is already configured, or the next extractor doesn't want to be spliced in, or
       // the next extractor isn't ready to be spliced in.
       // the next extractor isn't ready to be spliced in.
@@ -107,11 +150,12 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
   /**
   /**
    * Gets the number of available tracks.
    * Gets the number of available tracks.
    * <p>
    * <p>
-   * This method should only be called after the extractor has been prepared.
+   * This method must only be called after the extractor has been prepared.
    *
    *
    * @return The number of available tracks.
    * @return The number of available tracks.
    */
    */
   public int getTrackCount() {
   public int getTrackCount() {
+    Assertions.checkState(isPrepared());
     return sampleQueues.size();
     return sampleQueues.size();
   }
   }
 
 
@@ -124,51 +168,14 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
    * @return The corresponding format.
    * @return The corresponding format.
    */
    */
   public MediaFormat getMediaFormat(int track) {
   public MediaFormat getMediaFormat(int track) {
+    Assertions.checkState(isPrepared());
     return sampleQueues.valueAt(track).getFormat();
     return sampleQueues.valueAt(track).getFormat();
   }
   }
 
 
   /**
   /**
-   * Whether the extractor is prepared.
-   *
-   * @return True if the extractor is prepared. False otherwise.
-   */
-  public boolean isPrepared() {
-    if (!prepared && tracksBuilt) {
-      for (int i = 0; i < sampleQueues.size(); i++) {
-        if (!sampleQueues.valueAt(i).hasFormat()) {
-          return false;
-        }
-      }
-      prepared = true;
-    }
-    return prepared;
-  }
-
-  /**
-   * Clears queues for all tracks, returning all allocations to the allocator.
-   */
-  public void clear() {
-    for (int i = 0; i < sampleQueues.size(); i++) {
-      sampleQueues.valueAt(i).clear();
-    }
-  }
-
-  /**
-   * Gets the largest timestamp of any sample parsed by the extractor.
-   *
-   * @return The largest timestamp, or {@link Long#MIN_VALUE} if no samples have been parsed.
-   */
-  public long getLargestParsedTimestampUs() {
-    long largestParsedTimestampUs = Long.MIN_VALUE;
-    for (int i = 0; i < sampleQueues.size(); i++) {
-      largestParsedTimestampUs = Math.max(largestParsedTimestampUs,
-          sampleQueues.valueAt(i).getLargestParsedTimestampUs());
-    }
-    return largestParsedTimestampUs;
-  }
-
-  /**
    * Gets the next sample for the specified track.
    * Gets the next sample for the specified track.
+   * <p>
+   * This method must only be called after the extractor has been prepared.
    *
    *
    * @param track The track from which to read.
    * @param track The track from which to read.
    * @param holder A {@link SampleHolder} into which the sample should be read.
    * @param holder A {@link SampleHolder} into which the sample should be read.
@@ -181,6 +188,8 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
 
 
   /**
   /**
    * Discards samples for the specified track up to the specified time.
    * Discards samples for the specified track up to the specified time.
+   * <p>
+   * This method must only be called after the extractor has been prepared.
    *
    *
    * @param track The track from which samples should be discarded.
    * @param track The track from which samples should be discarded.
    * @param timeUs The time up to which samples should be discarded, in microseconds.
    * @param timeUs The time up to which samples should be discarded, in microseconds.
@@ -193,6 +202,8 @@ public final class HlsExtractorWrapper implements ExtractorOutput {
   /**
   /**
    * Whether samples are available for reading from {@link #getSample(int, SampleHolder)} for the
    * Whether samples are available for reading from {@link #getSample(int, SampleHolder)} for the
    * specified track.
    * specified track.
+   * <p>
+   * This method must only be called after the extractor has been prepared.
    *
    *
    * @return True if samples are available for reading from {@link #getSample(int, SampleHolder)}
    * @return True if samples are available for reading from {@link #getSample(int, SampleHolder)}
    *     for the specified track. False otherwise.
    *     for the specified track. False otherwise.

library/src/main/java/com/google/android/exoplayer/hls/HlsSampleSource.java

@@ -80,9 +80,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
   private long lastSeekPositionUs;
   private long lastSeekPositionUs;
   private long pendingResetPositionUs;
   private long pendingResetPositionUs;
 
 
-  private TsChunk previousTsLoadable;
-  private Chunk currentLoadable;
   private boolean loadingFinished;
   private boolean loadingFinished;
+  private Chunk currentLoadable;
+  private TsChunk currentTsLoadable;
+  private TsChunk previousTsLoadable;
 
 
   private Loader loader;
   private Loader loader;
   private IOException currentLoadableException;
   private IOException currentLoadableException;
@@ -259,6 +260,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
     }
     }
 
 
     HlsExtractorWrapper extractor = getCurrentExtractor();
     HlsExtractorWrapper extractor = getCurrentExtractor();
+    if (!extractor.isPrepared()) {
+      maybeThrowLoadableException();
+      return NOTHING_READ;
+    }
 
 
     if (downstreamFormat == null || !downstreamFormat.equals(extractor.format)) {
     if (downstreamFormat == null || !downstreamFormat.equals(extractor.format)) {
       // Notify a change in the downstream format.
       // Notify a change in the downstream format.
@@ -277,11 +282,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
       // We're finished reading from the extractor for this particular track, so advance to the
       // We're finished reading from the extractor for this particular track, so advance to the
       // next one for the current read.
       // next one for the current read.
       extractor = extractors.get(++extractorIndex);
       extractor = extractors.get(++extractorIndex);
-    }
-
-    if (!extractor.isPrepared()) {
-      maybeThrowLoadableException();
-      return NOTHING_READ;
+      if (!extractor.isPrepared()) {
+        maybeThrowLoadableException();
+        return NOTHING_READ;
+      }
     }
     }
 
 
     MediaFormat mediaFormat = extractor.getMediaFormat(track);
     MediaFormat mediaFormat = extractor.getMediaFormat(track);
@@ -351,6 +355,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
   public void onLoadCompleted(Loadable loadable) {
   public void onLoadCompleted(Loadable loadable) {
     long now = SystemClock.elapsedRealtime();
     long now = SystemClock.elapsedRealtime();
     long loadDurationMs = now - currentLoadStartTimeMs;
     long loadDurationMs = now - currentLoadStartTimeMs;
+    if (currentTsLoadable != null) {
+      previousTsLoadable = currentTsLoadable;
+      currentTsLoadable = null;
+    }
     chunkSource.onChunkLoadCompleted(currentLoadable);
     chunkSource.onChunkLoadCompleted(currentLoadable);
     if (isTsChunk(currentLoadable)) {
     if (isTsChunk(currentLoadable)) {
       TsChunk tsChunk = (TsChunk) loadable;
       TsChunk tsChunk = (TsChunk) loadable;
@@ -462,6 +470,7 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
   }
   }
 
 
   private void clearCurrentLoadable() {
   private void clearCurrentLoadable() {
+    currentTsLoadable = null;
     currentLoadable = null;
     currentLoadable = null;
     currentLoadableException = null;
     currentLoadableException = null;
     currentLoadableExceptionCount = 0;
     currentLoadableExceptionCount = 0;
@@ -510,7 +519,7 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
       }
       }
       notifyLoadStarted(tsChunk.dataSpec.length, tsChunk.type, tsChunk.trigger, tsChunk.format,
       notifyLoadStarted(tsChunk.dataSpec.length, tsChunk.type, tsChunk.trigger, tsChunk.format,
           tsChunk.startTimeUs, tsChunk.endTimeUs);
           tsChunk.startTimeUs, tsChunk.endTimeUs);
-      previousTsLoadable = tsChunk;
+      currentTsLoadable = tsChunk;
     } else {
     } else {
       notifyLoadStarted(currentLoadable.dataSpec.length, currentLoadable.type,
       notifyLoadStarted(currentLoadable.dataSpec.length, currentLoadable.type,
           currentLoadable.trigger, currentLoadable.format, -1, -1);
           currentLoadable.trigger, currentLoadable.format, -1, -1);
@@ -525,6 +534,8 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader
   private long getNextLoadPositionUs() {
   private long getNextLoadPositionUs() {
     if (isPendingReset()) {
     if (isPendingReset()) {
       return pendingResetPositionUs;
       return pendingResetPositionUs;
+    } else if (loader.isLoading()) {
+      return currentTsLoadable.isLastChunk ? -1 : currentTsLoadable.endTimeUs;
     } else {
     } else {
       return previousTsLoadable.isLastChunk ? -1 : previousTsLoadable.endTimeUs;
       return previousTsLoadable.isLastChunk ? -1 : previousTsLoadable.endTimeUs;
     }
     }