Update with first round of code review suggestions

Update with code review suggestions: 1. rename to HlsCheckedSampleQueue to HlsSampleQueue and combine with format adjusting class. 2. Copywrite in added classes 3. Capture additional items useful for recovery and reporting in the exception class 4. Remove extraneous logging 5. eliminate magic number (50 seconds) and use percentage of duration WIP: - eliminate null check for chunk (chunkless prepare starts load before sampleQueue are created) - work out recovery strategy
2025-05-15 11:39:56 +08:00 · 2020-03-13 09:59:27 -07:00 · 2020-03-13 09:59:27 -07:00 · e690979d92
commit e690979d92
parent f78cbd2c9e
6 changed files with 236 additions and 177 deletions
--- a/library/core/src/main/java/com/google/android/exoplayer2/source/UnexpectedDiscontinuityException.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/source/UnexpectedDiscontinuityException.java
@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2.source;
+
+import com.google.android.exoplayer2.C;
+import com.google.android.exoplayer2.source.chunk.MediaChunk;
+import com.google.android.exoplayer2.upstream.DataSpec;
+
+/**
+ * Thrown from the loader thread when an attempt is made to commit a sample that is far
+ * deviant from the expected sequence of timestamps in the SampleStream.
+ *
+ * This is likely caused by an intra-chunk timestamp discontinuity that was not handled by the
+ * chunk source (the origin server).
+ *
+ * For HLS, the origin server is required to break segments at continuity boundaries by the HLS Pantos spec
+ * (EXT-X-DISCONTINUITY {@see https://tools.ietf.org/html/draft-pantos-hls-rfc8216bis-04#section-4.4.2.3})
+ *
+ */
+public final class UnexpectedDiscontinuityException extends RuntimeException {
+
+  /** the last in-bounds timestamp committed to the {@link SampleQueue}, or
+   * {@link C#TIME_UNSET} if this was for the first committed sample
+   */
+  public final long lastValidTimeUs;
+
+  /** The errant timestamp
+   */
+  public final long deviantSampleTimeUs;
+
+  /** The source of the samples that resulted in this error
+   */
+  public final DataSpec dataSpec;
+
+  /** The timeUs that the source of the samples starts (from HLS metadata)
+   */
+  public final long startTimeUs;
+
+  /**
+   * Construct an UnexpectedDiscontinuityException for a {@link MediaChunk} where an
+   * unexpected timestamp discontinuity is detected within its sample source (e.g. segment for HLS)
+   *
+   * @param mediaChunk the {@link MediaChunk} with the unexpected timestamp value
+   * @param lastValidTimeUs the last in-bounds timestamp committed to the {@link SampleQueue}, or
+   *                        {@link C#TIME_UNSET} if this was for the first committed sample
+   * @param deviantSampleTimeUs the timestamp that is out of bounds.
+   */
+  public UnexpectedDiscontinuityException(MediaChunk mediaChunk, long lastValidTimeUs, long deviantSampleTimeUs) {
+    super("Unexpected discontinuity, timeMs: " + C.usToMs(deviantSampleTimeUs) + " loaded from dataSpec: " + mediaChunk.dataSpec);
+    this.dataSpec = mediaChunk.dataSpec;
+    this.startTimeUs = mediaChunk.startTimeUs;
+    this.lastValidTimeUs = lastValidTimeUs;
+    this.deviantSampleTimeUs = deviantSampleTimeUs;
+  }
+}
--- a/library/core/src/main/java/com/google/android/exoplayer2/source/UnreportedDiscontinuityException.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/source/UnreportedDiscontinuityException.java
@ -1,26 +0,0 @@
-package com.google.android.exoplayer2.source;
-
-import android.net.Uri;
-import com.google.android.exoplayer2.C;
-
-/**
- * Thrown from the loader thread when an attempt is made to commit a sample that is far
- * deviant from the expected sequence of timestamps in the SampleStream.   This is likely
- * caused by a discontinuity in a segment that was not split and reported by metadata in
- * an HLS (EXT-X-DISCONTINUITY) or DASH stream.
- */
-public class UnreportedDiscontinuityException extends RuntimeException {
-
-  public final long timesUs;
-
-  /**
-   * Consturct the exception
-   *
-   * @param timesUs last timestamp before attempted commit of the deviant sample
-   * @param uri uri of the segment with the unreported discontinuity
-   */
-  public UnreportedDiscontinuityException(long timesUs, Uri uri) {
-    super("Unreported discontinuity timeMs: " + C.usToMs(timesUs) + " in URI: " + uri);
-    this.timesUs = timesUs;
-  }
-}
--- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsCheckedSampleQueue.java
+++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsCheckedSampleQueue.java
@ -1,65 +0,0 @@
-/*
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.google.android.exoplayer2.source.hls;
-
-import androidx.annotation.Nullable;
-import com.google.android.exoplayer2.C;
-import com.google.android.exoplayer2.drm.DrmSessionManager;
-import com.google.android.exoplayer2.source.SampleQueue;
-import com.google.android.exoplayer2.source.UnreportedDiscontinuityException;
-import com.google.android.exoplayer2.upstream.Allocator;
-import com.google.android.exoplayer2.util.Log;
-
-public class HlsCheckedSampleQueue extends SampleQueue {
-  private static final String TAG = "HlsCheckedSampleQueue";
-
-  private long lowestTimeUs = C.TIME_UNSET;
-  private long highestTimeUs = C.TIME_UNSET;
-
-  private HlsMediaChunk chunk;
-  private boolean loggedFirst = false;
-
-  HlsCheckedSampleQueue(Allocator allocator, DrmSessionManager drmSessionManager) {
-    super(allocator, drmSessionManager);
-  }
-
-  void setCurrentLoadingChunk(HlsMediaChunk chunk) {
-    double tolerance = (chunk.endTimeUs - chunk.startTimeUs) * 0.1;
-    this.lowestTimeUs = chunk.startTimeUs;
-    this.highestTimeUs = (long) (chunk.endTimeUs + tolerance);
-    this.chunk = chunk;
-    loggedFirst = false;
-  }
-
-
-  @Override
-  public void sampleMetadata(long timeUs, int flags, int size, int offset,  @Nullable CryptoData cryptoData) {
-    if (lowestTimeUs != C.TIME_UNSET && timeUs < lowestTimeUs && ! loggedFirst) {
-      Log.d(TAG, "sampleMetadata() - committed timeUs: " + timeUs + " is " + C.usToMs(lowestTimeUs - timeUs) + "ms less then segment start time.  chunk: " + chunk.dataSpec.uri);
-      loggedFirst = true;
-    }
-    if (lowestTimeUs != C.TIME_UNSET && timeUs < (lowestTimeUs - C.msToUs(50_000))) {
-      Log.d(TAG, "sampleMetadata() - committed timeUs: " + timeUs + " is " + C.usToMs(lowestTimeUs - timeUs) + "ms less (MUCH!) then segment start time.  chunk: " + chunk.dataSpec.uri);
-      throw new UnreportedDiscontinuityException(timeUs, chunk.dataSpec.uri);
-    }
-    if (highestTimeUs != C.TIME_UNSET && timeUs > highestTimeUs) {
-      Log.d(TAG, "sampleMetadata() - committed timeUs: " + timeUs + " is " + C.usToMs(lowestTimeUs - timeUs) + "ms greater then segment end time.  chunk: " + chunk.dataSpec.uri);
-      throw new UnreportedDiscontinuityException(timeUs, chunk.dataSpec.uri);
-    }
-    super.sampleMetadata(timeUs, flags, size, offset, cryptoData);
-  }
-
-}
--- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java
+++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java
@ -27,7 +27,7 @@ import com.google.android.exoplayer2.extractor.PositionHolder;
 import com.google.android.exoplayer2.metadata.Metadata;
 import com.google.android.exoplayer2.metadata.id3.Id3Decoder;
 import com.google.android.exoplayer2.metadata.id3.PrivFrame;
-import com.google.android.exoplayer2.source.UnreportedDiscontinuityException;
+import com.google.android.exoplayer2.source.UnexpectedDiscontinuityException;
 import com.google.android.exoplayer2.source.chunk.MediaChunk;
 import com.google.android.exoplayer2.source.hls.playlist.HlsMediaPlaylist;
 import com.google.android.exoplayer2.upstream.DataSource;
@ -379,10 +379,9 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
        while (result == Extractor.RESULT_CONTINUE && !loadCanceled) {
          result = extractor.read(input, DUMMY_POSITION_HOLDER);
        }
-      } catch (UnreportedDiscontinuityException e) {
-        Log.d(TAG, "Unreported discontinuity at timeUs: "+ e.timesUs + " uri: " + dataSpec.uri);
-        throw new IOException("Timestamp error", e);
-        
+      } catch (UnexpectedDiscontinuityException e) {
+        Log.d(TAG, "UnexpectedDiscontinuityException - recovering by discarding balance of segment", e);
+        throw new IOException("load aborted for segment - " + e.dataSpec + " unexpected discontinuity", e);
      } finally {
        nextLoadPosition = (int) (input.getPosition() - dataSpec.position);
      }
--- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleQueue.java
+++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleQueue.java
@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2.source.hls;
+
+import androidx.annotation.Nullable;
+import com.google.android.exoplayer2.C;
+import com.google.android.exoplayer2.Format;
+import com.google.android.exoplayer2.drm.DrmInitData;
+import com.google.android.exoplayer2.drm.DrmSessionManager;
+import com.google.android.exoplayer2.metadata.Metadata;
+import com.google.android.exoplayer2.metadata.id3.PrivFrame;
+import com.google.android.exoplayer2.source.SampleQueue;
+import com.google.android.exoplayer2.source.UnexpectedDiscontinuityException;
+import com.google.android.exoplayer2.upstream.Allocator;
+import com.google.android.exoplayer2.util.Assertions;
+import com.google.android.exoplayer2.util.Log;
+import com.google.android.exoplayer2.util.MediaSourceEventDispatcher;
+import com.google.android.exoplayer2.util.TimestampAdjuster;
+import java.util.Map;
+
+/**
+ * Extend base SampleQueue to add HLS specific processing of the samples, including:
+ *
+ * <ul>
+ *   <li>segment time boundary checks on timestamps of committed samples</li>
+ *   <li>cleaning the {@link Format#metadata} to avoid excessive format changes</li>
+ * </ul>
+ *
+ * The timestamp check verifies that the adjusted sample time (via {@link TimestampAdjuster}) does not
+ * fall outside of a set percentage ({@link #MAX_TIMESTAMP_DEVIATION_PERCENTAGE}) of the time
+ * boundaries of the segment as expressed by the segment duration ((@link HlsMediaChunk#endTimeUs} -
+ * {@link HlsMediaChunk#startTimeUs}).  This is loosely mandated by the Pantos spec and checked by
+ * Apple's mediastreamvalidator.
+ *
+ */
+public class HlsSampleQueue extends SampleQueue {
+
+  private static final String TAG = "HlsSampleQueue";
+
+  /**
+   * largest timestamp deviation from the segment time bounds expressed as a percentage of
+   * the segment duration.
+   */
+  public static double MAX_TIMESTAMP_DEVIATION_PERCENTAGE = 0.50;
+
+  private long lowestTimeUs = C.TIME_UNSET;
+  private long highestTimeUs = C.TIME_UNSET;
+
+  @Nullable private HlsMediaChunk chunk;
+  private long lastValidTimeUs;
+
+  private final Map<String, DrmInitData> overridingDrmInitData;
+  @Nullable private DrmInitData drmInitData;
+
+  public HlsSampleQueue(Allocator allocator,
+          DrmSessionManager<?> drmSessionManager,
+          MediaSourceEventDispatcher eventDispatcher,
+          Map<String, DrmInitData> overridingDrmInitData) {
+    super(allocator, drmSessionManager, eventDispatcher);
+    this.overridingDrmInitData = overridingDrmInitData;
+  }
+
+  void setCurrentLoadingChunk(HlsMediaChunk chunk) {
+    double tolerance = (chunk.endTimeUs - chunk.startTimeUs) * MAX_TIMESTAMP_DEVIATION_PERCENTAGE;
+    this.lowestTimeUs = (long) (chunk.startTimeUs - tolerance);
+    this.highestTimeUs = (long) (chunk.endTimeUs + tolerance);
+    this.chunk = chunk;
+    lastValidTimeUs = C.TIME_UNSET;
+  }
+
+  public void setDrmInitData(@Nullable DrmInitData drmInitData) {
+    this.drmInitData = drmInitData;
+    invalidateUpstreamFormatAdjustment();
+  }
+
+  @SuppressWarnings("ReferenceEquality")
+  @Override
+  public Format getAdjustedUpstreamFormat(Format format) {
+    @Nullable
+    DrmInitData drmInitData = this.drmInitData != null ? this.drmInitData : format.drmInitData;
+    if (drmInitData != null) {
+      @Nullable
+      DrmInitData overridingDrmInitData = this.overridingDrmInitData.get(drmInitData.schemeType);
+      if (overridingDrmInitData != null) {
+        drmInitData = overridingDrmInitData;
+      }
+    }
+    @Nullable Metadata metadata = getAdjustedMetadata(format.metadata);
+    if (drmInitData != format.drmInitData || metadata != format.metadata) {
+      format = format.buildUpon().setDrmInitData(drmInitData).setMetadata(metadata).build();
+    }
+    return super.getAdjustedUpstreamFormat(format);
+  }
+
+  /**
+   * Strips the private timestamp frame from metadata, if present. See:
+   * https://github.com/google/ExoPlayer/issues/5063
+   */
+  @Nullable
+  private Metadata getAdjustedMetadata(@Nullable Metadata metadata) {
+    if (metadata == null) {
+      return null;
+    }
+    int length = metadata.length();
+    int transportStreamTimestampMetadataIndex = C.INDEX_UNSET;
+    for (int i = 0; i < length; i++) {
+      Metadata.Entry metadataEntry = metadata.get(i);
+      if (metadataEntry instanceof PrivFrame) {
+        PrivFrame privFrame = (PrivFrame) metadataEntry;
+        if (HlsMediaChunk.PRIV_TIMESTAMP_FRAME_OWNER.equals(privFrame.owner)) {
+          transportStreamTimestampMetadataIndex = i;
+          break;
+        }
+      }
+    }
+    if (transportStreamTimestampMetadataIndex == C.INDEX_UNSET) {
+      return metadata;
+    }
+    if (length == 1) {
+      return null;
+    }
+    Metadata.Entry[] newMetadataEntries = new Metadata.Entry[length - 1];
+    for (int i = 0; i < length; i++) {
+      if (i != transportStreamTimestampMetadataIndex) {
+        int newIndex = i < transportStreamTimestampMetadataIndex ? i : i - 1;
+        newMetadataEntries[newIndex] = metadata.get(i);
+      }
+    }
+    return new Metadata(newMetadataEntries);
+  }
+
+  @Override
+  public void sampleMetadata(long timeUs, int flags, int size, int offset,  @Nullable CryptoData cryptoData) {
+    // TODO - chunkless prepare, sampleQueue list is not yet initialized for first chunk
+//    Assertions.checkNotNull(chunk, "sampleMetadata without a MediaChunk?");
+    if (chunk == null) {
+      super.sampleMetadata(timeUs, flags, size, offset, cryptoData);
+    } else if (timeUs > highestTimeUs || timeUs < lowestTimeUs) {
+      throw new UnexpectedDiscontinuityException(chunk, lastValidTimeUs, timeUs);
+    } else {
+      lastValidTimeUs = timeUs;
+      super.sampleMetadata(timeUs, flags, size, offset, cryptoData);
+    }
+  }
+
+}
--- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java
+++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java
@ -129,7 +129,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
  private final ArrayList<HlsSampleStream> hlsSampleStreams;
  private final Map<String, DrmInitData> overridingDrmInitData;

-  private FormatAdjustingSampleQueue[] sampleQueues;
+  private HlsSampleQueue[] sampleQueues;
  private int[] sampleQueueTrackIds;
  private Set<Integer> sampleQueueMappingDoneByType;
  private SparseIntArray sampleQueueIndicesByType;
@ -209,7 +209,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
    sampleQueueTrackIds = new int[0];
    sampleQueueMappingDoneByType = new HashSet<>(MAPPABLE_TYPES.size());
    sampleQueueIndicesByType = new SparseIntArray(MAPPABLE_TYPES.size());
-    sampleQueues = new FormatAdjustingSampleQueue[0];
+    sampleQueues = new HlsSampleQueue[0];
    sampleQueueIsAudioVideoFlags = new boolean[0];
    sampleQueuesEnabledStates = new boolean[0];
    mediaChunks = new ArrayList<>();
@ -827,7 +827,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
   */
  public void init(int chunkUid, HlsMediaChunk loadingChunk, boolean shouldSpliceIn) {
    this.chunkUid = chunkUid;
-    for (HlsCheckedSampleQueue sampleQueue : sampleQueues) {
+    for (HlsSampleQueue sampleQueue : sampleQueues) {
      sampleQueue.sourceId(chunkUid);
      sampleQueue.setCurrentLoadingChunk(loadingChunk);
    }
@ -908,8 +908,8 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
    int trackCount = sampleQueues.length;

    boolean isAudioVideo = type == C.TRACK_TYPE_AUDIO || type == C.TRACK_TYPE_VIDEO;
-    FormatAdjustingSampleQueue trackOutput =
-        new FormatAdjustingSampleQueue(
+    HlsSampleQueue trackOutput =
+        new HlsSampleQueue(
            allocator, drmSessionManager, eventDispatcher, overridingDrmInitData);
    if (isAudioVideo) {
      trackOutput.setDrmInitData(drmInitData);
@ -1343,82 +1343,6 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
    return new DummyTrackOutput();
  }

-  private static final class FormatAdjustingSampleQueue extends HlsCheckedSampleQueue {
-
-    private final Map<String, DrmInitData> overridingDrmInitData;
-    @Nullable private DrmInitData drmInitData;
-
-    public FormatAdjustingSampleQueue(
-        Allocator allocator,
-        DrmSessionManager<?> drmSessionManager,
-        MediaSourceEventDispatcher eventDispatcher,
-        Map<String, DrmInitData> overridingDrmInitData) {
-      super(allocator, drmSessionManager, eventDispatcher);
-      this.overridingDrmInitData = overridingDrmInitData;
-    }
-
-    public void setDrmInitData(@Nullable DrmInitData drmInitData) {
-      this.drmInitData = drmInitData;
-      invalidateUpstreamFormatAdjustment();
-    }
-
-    @SuppressWarnings("ReferenceEquality")
-    @Override
-    public Format getAdjustedUpstreamFormat(Format format) {
-      @Nullable
-      DrmInitData drmInitData = this.drmInitData != null ? this.drmInitData : format.drmInitData;
-      if (drmInitData != null) {
-        @Nullable
-        DrmInitData overridingDrmInitData = this.overridingDrmInitData.get(drmInitData.schemeType);
-        if (overridingDrmInitData != null) {
-          drmInitData = overridingDrmInitData;
-        }
-      }
-      @Nullable Metadata metadata = getAdjustedMetadata(format.metadata);
-      if (drmInitData != format.drmInitData || metadata != format.metadata) {
-        format = format.buildUpon().setDrmInitData(drmInitData).setMetadata(metadata).build();
-      }
-      return super.getAdjustedUpstreamFormat(format);
-    }
-
-    /**
-     * Strips the private timestamp frame from metadata, if present. See:
-     * https://github.com/google/ExoPlayer/issues/5063
-     */
-    @Nullable
-    private Metadata getAdjustedMetadata(@Nullable Metadata metadata) {
-      if (metadata == null) {
-        return null;
-      }
-      int length = metadata.length();
-      int transportStreamTimestampMetadataIndex = C.INDEX_UNSET;
-      for (int i = 0; i < length; i++) {
-        Metadata.Entry metadataEntry = metadata.get(i);
-        if (metadataEntry instanceof PrivFrame) {
-          PrivFrame privFrame = (PrivFrame) metadataEntry;
-          if (HlsMediaChunk.PRIV_TIMESTAMP_FRAME_OWNER.equals(privFrame.owner)) {
-            transportStreamTimestampMetadataIndex = i;
-            break;
-          }
-        }
-      }
-      if (transportStreamTimestampMetadataIndex == C.INDEX_UNSET) {
-        return metadata;
-      }
-      if (length == 1) {
-        return null;
-      }
-      Metadata.Entry[] newMetadataEntries = new Metadata.Entry[length - 1];
-      for (int i = 0; i < length; i++) {
-        if (i != transportStreamTimestampMetadataIndex) {
-          int newIndex = i < transportStreamTimestampMetadataIndex ? i : i - 1;
-          newMetadataEntries[newIndex] = metadata.get(i);
-        }
-      }
-      return new Metadata(newMetadataEntries);
-    }
-  }
-
  private static class EmsgUnwrappingTrackOutput implements TrackOutput {

    private static final String TAG = "EmsgUnwrappingTrackOutput";