From e690979d92e05e90ed8971e9bf733a7ec97e5245 Mon Sep 17 00:00:00 2001 From: Steve Mayhew Date: Fri, 13 Mar 2020 09:59:27 -0700 Subject: [PATCH] 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 --- .../UnexpectedDiscontinuityException.java | 68 ++++++++ .../UnreportedDiscontinuityException.java | 26 --- .../source/hls/HlsCheckedSampleQueue.java | 65 ------- .../exoplayer2/source/hls/HlsMediaChunk.java | 9 +- .../exoplayer2/source/hls/HlsSampleQueue.java | 159 ++++++++++++++++++ .../source/hls/HlsSampleStreamWrapper.java | 86 +--------- 6 files changed, 236 insertions(+), 177 deletions(-) create mode 100644 library/core/src/main/java/com/google/android/exoplayer2/source/UnexpectedDiscontinuityException.java delete mode 100644 library/core/src/main/java/com/google/android/exoplayer2/source/UnreportedDiscontinuityException.java delete mode 100644 library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsCheckedSampleQueue.java create mode 100644 library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleQueue.java diff --git 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 new file mode 100644 index 0000000000..d6033fef2d --- /dev/null +++ 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; + } +} diff --git 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 deleted file mode 100644 index 9ed2177e1e..0000000000 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/UnreportedDiscontinuityException.java +++ /dev/null @@ -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; - } -} diff --git 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 deleted file mode 100644 index 7630d40680..0000000000 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsCheckedSampleQueue.java +++ /dev/null @@ -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); - } - -} diff --git 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 index a2e4840402..e700068321 100644 --- 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); } diff --git 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 new file mode 100644 index 0000000000..c2e22b8344 --- /dev/null +++ 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: + * + * + * + * 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 overridingDrmInitData; + @Nullable private DrmInitData drmInitData; + + public HlsSampleQueue(Allocator allocator, + DrmSessionManager drmSessionManager, + MediaSourceEventDispatcher eventDispatcher, + Map 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); + } + } + +} diff --git 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 index 434d30448b..9c7d5f889e 100644 --- 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 hlsSampleStreams; private final Map overridingDrmInitData; - private FormatAdjustingSampleQueue[] sampleQueues; + private HlsSampleQueue[] sampleQueues; private int[] sampleQueueTrackIds; private Set 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 overridingDrmInitData; - @Nullable private DrmInitData drmInitData; - - public FormatAdjustingSampleQueue( - Allocator allocator, - DrmSessionManager drmSessionManager, - MediaSourceEventDispatcher eventDispatcher, - Map 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";