From 16e6ea6e40de35676a03d09b2c1b9807eae9b6bc Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 12 Mar 2020 10:22:53 +0000 Subject: [PATCH] Fix spurious reset of PreparedState boolean flags PiperOrigin-RevId: 300513930 --- .../source/ProgressiveMediaPeriod.java | 87 ++++++++++--------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java index c7387efd34..9c60280dd4 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java @@ -57,6 +57,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.checkerframework.checker.nullness.compatqual.NullableType; +import org.checkerframework.checker.nullness.qual.EnsuresNonNull; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** A {@link MediaPeriod} that extracts data using an {@link Extractor}. */ @@ -111,23 +112,24 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private final Handler handler; @Nullable private Callback callback; - @Nullable private SeekMap seekMap; @Nullable private IcyHeaders icyHeaders; private SampleQueue[] sampleQueues; private TrackId[] sampleQueueTrackIds; private boolean sampleQueuesBuilt; - private @MonotonicNonNull PreparedState preparedState; + private boolean prepared; private boolean haveAudioVideoTracks; + private @MonotonicNonNull TrackState trackState; + private @MonotonicNonNull SeekMap seekMap; + private long durationUs; + private boolean isLive; private int dataType; private boolean seenFirstTrackSelection; private boolean notifyDiscontinuity; private boolean notifiedReadingStarted; private int enabledTrackCount; - private long durationUs; private long length; - private boolean isLive; private long lastSeekPositionUs; private long pendingResetPositionUs; @@ -197,7 +199,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } public void release() { - if (preparedState != null) { + if (prepared) { // Discard as much as we can synchronously. We only do this if we're prepared, since otherwise // sampleQueues may still be being modified by the loading thread. for (SampleQueue sampleQueue : sampleQueues) { @@ -229,14 +231,15 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void maybeThrowPrepareError() throws IOException { maybeThrowError(); - if (loadingFinished && preparedState == null) { + if (loadingFinished && !prepared) { throw new ParserException("Loading finished before preparation is complete."); } } @Override public TrackGroupArray getTrackGroups() { - return Assertions.checkNotNull(preparedState).tracks; + assertPrepared(); + return trackState.tracks; } @Override @@ -246,8 +249,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @NullableType SampleStream[] streams, boolean[] streamResetFlags, long positionUs) { - TrackGroupArray tracks = Assertions.checkNotNull(preparedState).tracks; - boolean[] trackEnabledStates = preparedState.trackEnabledStates; + assertPrepared(); + TrackGroupArray tracks = trackState.tracks; + boolean[] trackEnabledStates = trackState.trackEnabledStates; int oldEnabledTrackCount = enabledTrackCount; // Deselect old tracks. for (int i = 0; i < selections.length; i++) { @@ -316,10 +320,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void discardBuffer(long positionUs, boolean toKeyframe) { + assertPrepared(); if (isPendingReset()) { return; } - boolean[] trackEnabledStates = Assertions.checkNotNull(preparedState).trackEnabledStates; + boolean[] trackEnabledStates = trackState.trackEnabledStates; int trackCount = sampleQueues.length; for (int i = 0; i < trackCount; i++) { sampleQueues[i].discardTo(positionUs, toKeyframe, trackEnabledStates[i]); @@ -336,7 +341,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; if (loadingFinished || loader.hasFatalError() || pendingDeferredRetry - || (preparedState != null && enabledTrackCount == 0)) { + || (prepared && enabledTrackCount == 0)) { return false; } boolean continuedLoading = loadCondition.open(); @@ -373,8 +378,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public long getBufferedPositionUs() { - boolean[] trackIsAudioVideoFlags = - Assertions.checkNotNull(preparedState).trackIsAudioVideoFlags; + assertPrepared(); + boolean[] trackIsAudioVideoFlags = trackState.trackIsAudioVideoFlags; if (loadingFinished) { return C.TIME_END_OF_SOURCE; } else if (isPendingReset()) { @@ -400,8 +405,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public long seekToUs(long positionUs) { - SeekMap seekMap = Assertions.checkNotNull(preparedState).seekMap; - boolean[] trackIsAudioVideoFlags = preparedState.trackIsAudioVideoFlags; + assertPrepared(); + boolean[] trackIsAudioVideoFlags = trackState.trackIsAudioVideoFlags; // Treat all seeks into non-seekable media as being to t=0. positionUs = seekMap.isSeekable() ? positionUs : 0; @@ -436,7 +441,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public long getAdjustedSeekPositionUs(long positionUs, SeekParameters seekParameters) { - SeekMap seekMap = Assertions.checkNotNull(preparedState).seekMap; + assertPrepared(); if (!seekMap.isSeekable()) { // Treat all seeks into non-seekable media as being to t=0. return 0; @@ -498,10 +503,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } private void maybeNotifyDownstreamFormat(int track) { - boolean[] trackNotifiedDownstreamFormats = - Assertions.checkNotNull(preparedState).trackNotifiedDownstreamFormats; + assertPrepared(); + boolean[] trackNotifiedDownstreamFormats = trackState.trackNotifiedDownstreamFormats; if (!trackNotifiedDownstreamFormats[track]) { - Format trackFormat = preparedState.tracks.get(track).getFormat(/* index= */ 0); + Format trackFormat = trackState.tracks.get(track).getFormat(/* index= */ 0); eventDispatcher.downstreamFormatChanged( MimeTypes.getTrackType(trackFormat.sampleMimeType), trackFormat, @@ -513,8 +518,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } private void maybeStartDeferredRetry(int track) { - boolean[] trackIsAudioVideoFlags = - Assertions.checkNotNull(preparedState).trackIsAudioVideoFlags; + assertPrepared(); + boolean[] trackIsAudioVideoFlags = trackState.trackIsAudioVideoFlags; if (!pendingDeferredRetry || !trackIsAudioVideoFlags[track] || sampleQueues[track].isReady(/* loadingFinished= */ false)) { @@ -688,12 +693,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } private void setSeekMap(SeekMap seekMap) { - this.seekMap = icyHeaders == null ? seekMap : new Unseekable(/* durationUs */ C.TIME_UNSET); - if (preparedState == null) { + this.seekMap = icyHeaders == null ? seekMap : new Unseekable(/* durationUs= */ C.TIME_UNSET); + if (!prepared) { maybeFinishPrepare(); - } else { - preparedState = - new PreparedState(seekMap, preparedState.tracks, preparedState.trackIsAudioVideoFlags); } durationUs = seekMap.getDurationUs(); isLive = length == C.LENGTH_UNSET && seekMap.getDurationUs() == C.TIME_UNSET; @@ -702,8 +704,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } private void maybeFinishPrepare() { - SeekMap seekMap = this.seekMap; - if (released || preparedState != null || !sampleQueuesBuilt || seekMap == null) { + if (released || prepared || !sampleQueuesBuilt || seekMap == null) { return; } for (SampleQueue sampleQueue : sampleQueues) { @@ -744,8 +745,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } trackArray[i] = new TrackGroup(trackFormat); } - preparedState = - new PreparedState(seekMap, new TrackGroupArray(trackArray), trackIsAudioVideoFlags); + trackState = new TrackState(new TrackGroupArray(trackArray), trackIsAudioVideoFlags); + prepared = true; Assertions.checkNotNull(callback).onPrepared(this); } @@ -759,8 +760,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ExtractingLoadable loadable = new ExtractingLoadable( uri, dataSource, progressiveMediaExtractor, /* extractorOutput= */ this, loadCondition); - if (preparedState != null) { - SeekMap seekMap = preparedState.seekMap; + if (prepared) { Assertions.checkState(isPendingReset()); if (durationUs != C.TIME_UNSET && pendingResetPositionUs > durationUs) { loadingFinished = true; @@ -768,7 +768,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return; } loadable.setLoadPosition( - seekMap.getSeekPoints(pendingResetPositionUs).first.position, pendingResetPositionUs); + Assertions.checkNotNull(seekMap).getSeekPoints(pendingResetPositionUs).first.position, + pendingResetPositionUs); pendingResetPositionUs = C.TIME_UNSET; } extractedSamplesCountAtStartOfLoad = getExtractedSamplesCount(); @@ -803,7 +804,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // request data starting from the point it left off. extractedSamplesCountAtStartOfLoad = currentExtractedSampleCount; return true; - } else if (preparedState != null && !suppressRead()) { + } else if (prepared && !suppressRead()) { // We're playing a stream of unknown length and duration. Assume it's live, and therefore that // the data at the uri is a continuously shifting window of the latest available media. For // this case there's no way to continue loading from where a previous load finished, so it's @@ -820,7 +821,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // because there's no buffered data to be read. This case also covers an on-demand stream with // unknown length that has yet to be prepared. This case cannot be disambiguated from the live // stream case, so we have no option but to load from the start. - notifyDiscontinuity = preparedState != null; + notifyDiscontinuity = prepared; lastSeekPositionUs = 0; extractedSamplesCountAtStartOfLoad = 0; for (SampleQueue sampleQueue : sampleQueues) { @@ -875,6 +876,13 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return pendingResetPositionUs != C.TIME_UNSET; } + @EnsuresNonNull({"trackState", "seekMap"}) + private void assertPrepared() { + Assertions.checkState(prepared); + Assertions.checkNotNull(trackState); + Assertions.checkNotNull(seekMap); + } + private final class SampleStreamImpl implements SampleStream { private final int track; @@ -1042,18 +1050,15 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } } - /** Stores state that is initialized when preparation completes. */ - private static final class PreparedState { + /** Stores track state. */ + private static final class TrackState { - public final SeekMap seekMap; public final TrackGroupArray tracks; public final boolean[] trackIsAudioVideoFlags; public final boolean[] trackEnabledStates; public final boolean[] trackNotifiedDownstreamFormats; - public PreparedState( - SeekMap seekMap, TrackGroupArray tracks, boolean[] trackIsAudioVideoFlags) { - this.seekMap = seekMap; + public TrackState(TrackGroupArray tracks, boolean[] trackIsAudioVideoFlags) { this.tracks = tracks; this.trackIsAudioVideoFlags = trackIsAudioVideoFlags; this.trackEnabledStates = new boolean[tracks.length];