Skip to content

Commit c64d9fd

Browse files
toniheiicbaker
tonihei
authored and
icbaker
committed
Fix race condition in clipped sample streams
The streams return end-of-input if they read no samples, but know that they are fully buffered to at least the clipped end time. This helps to detect the end of stream even if there are no new buffers after the end of the clip (e.g. for sparse metadata tracks). The race condition occurs because the buffered position is evaluated after reading the sample. So between reading "no sample" and checking the buffered position, the source may have loaded arbitrary amounts of data. This may lead to a situation where the source has not read all samples, reads NOTHING_READ (because the queue is empty) and then immediately returns end-of-stream (because the buffered position jumped forward), causing all remaining samples in the stream to be skipped. This can fixed by moving the buffered position check to before reading the sample, so that it never exceeds the buffered position at the time of reading "no sample". #minor-release PiperOrigin-RevId: 548646464
1 parent 89972db commit c64d9fd

File tree

4 files changed

+293
-2
lines changed

4 files changed

+293
-2
lines changed

‎libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ClippingMediaPeriod.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ public int readData(
327327
buffer.setFlags(C.BUFFER_FLAG_END_OF_STREAM);
328328
return C.RESULT_BUFFER_READ;
329329
}
330+
long bufferedPositionUs = getBufferedPositionUs();
330331
@ReadDataResult int result = childStream.readData(formatHolder, buffer, readFlags);
331332
if (result == C.RESULT_FORMAT_READ) {
332333
Format format = Assertions.checkNotNull(formatHolder.format);
@@ -346,7 +347,7 @@ public int readData(
346347
if (endUs != C.TIME_END_OF_SOURCE
347348
&& ((result == C.RESULT_BUFFER_READ && buffer.timeUs >= endUs)
348349
|| (result == C.RESULT_NOTHING_READ
349-
&& getBufferedPositionUs() == C.TIME_END_OF_SOURCE
350+
&& bufferedPositionUs == C.TIME_END_OF_SOURCE
350351
&& !buffer.waitingForKeys))) {
351352
buffer.clear();
352353
buffer.setFlags(C.BUFFER_FLAG_END_OF_STREAM);

‎libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSource.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -879,14 +879,15 @@ public long selectTracks(
879879
@SampleStream.ReadFlags int readFlags) {
880880
@SampleStream.ReadFlags
881881
int peekingFlags = readFlags | SampleStream.FLAG_PEEK | SampleStream.FLAG_OMIT_SAMPLE_DATA;
882+
long bufferedPositionUs = getBufferedPositionUs(mediaPeriod);
882883
@SampleStream.ReadDataResult
883884
int result =
884885
castNonNull(sampleStreams[streamIndex]).readData(formatHolder, buffer, peekingFlags);
885886
long adjustedTimeUs =
886887
getMediaPeriodPositionUsWithEndOfSourceHandling(mediaPeriod, buffer.timeUs);
887888
if ((result == C.RESULT_BUFFER_READ && adjustedTimeUs == C.TIME_END_OF_SOURCE)
888889
|| (result == C.RESULT_NOTHING_READ
889-
&& getBufferedPositionUs(mediaPeriod) == C.TIME_END_OF_SOURCE
890+
&& bufferedPositionUs == C.TIME_END_OF_SOURCE
890891
&& !buffer.waitingForKeys)) {
891892
maybeNotifyDownstreamFormatChanged(mediaPeriod, streamIndex);
892893
buffer.clear();

‎libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ClippingMediaPeriodTest.java

+146
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright 2023 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package androidx.media3.exoplayer.source;
17+
18+
import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.END_OF_STREAM_ITEM;
19+
import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.oneByteSample;
20+
import static com.google.common.truth.Truth.assertThat;
21+
22+
import androidx.annotation.Nullable;
23+
import androidx.media3.common.C;
24+
import androidx.media3.common.Format;
25+
import androidx.media3.common.TrackGroup;
26+
import androidx.media3.decoder.DecoderInputBuffer;
27+
import androidx.media3.exoplayer.FormatHolder;
28+
import androidx.media3.exoplayer.drm.DrmSessionEventListener;
29+
import androidx.media3.exoplayer.drm.DrmSessionManager;
30+
import androidx.media3.exoplayer.trackselection.ExoTrackSelection;
31+
import androidx.media3.exoplayer.trackselection.FixedTrackSelection;
32+
import androidx.media3.exoplayer.upstream.Allocator;
33+
import androidx.media3.exoplayer.upstream.DefaultAllocator;
34+
import androidx.media3.test.utils.FakeMediaPeriod;
35+
import androidx.media3.test.utils.FakeSampleStream;
36+
import androidx.media3.test.utils.robolectric.RobolectricUtil;
37+
import androidx.test.ext.junit.runners.AndroidJUnit4;
38+
import com.google.common.collect.ImmutableList;
39+
import java.util.ArrayList;
40+
import java.util.List;
41+
import java.util.concurrent.atomic.AtomicBoolean;
42+
import org.junit.Test;
43+
import org.junit.runner.RunWith;
44+
45+
/** Unit tests for {@link ClippingMediaPeriod}. */
46+
@RunWith(AndroidJUnit4.class)
47+
public class ClippingMediaPeriodTest {
48+
49+
@Test
50+
public void fastLoadingStreamAfterFirstRead_canBeReadFully() throws Exception {
51+
TrackGroup trackGroup = new TrackGroup(new Format.Builder().build());
52+
// Set up MediaPeriod with no samples and only add samples after the first SampleStream read.
53+
FakeMediaPeriod mediaPeriod =
54+
new FakeMediaPeriod(
55+
new TrackGroupArray(trackGroup),
56+
new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024),
57+
/* trackDataFactory= */ (format, mediaPeriodId) -> ImmutableList.of(),
58+
new MediaSourceEventListener.EventDispatcher()
59+
.withParameters(
60+
/* windowIndex= */ 0,
61+
new MediaSource.MediaPeriodId(/* periodUid= */ new Object())),
62+
DrmSessionManager.DRM_UNSUPPORTED,
63+
new DrmSessionEventListener.EventDispatcher(),
64+
/* deferOnPrepared= */ false) {
65+
@Override
66+
protected FakeSampleStream createSampleStream(
67+
Allocator allocator,
68+
@Nullable MediaSourceEventListener.EventDispatcher mediaSourceEventDispatcher,
69+
DrmSessionManager drmSessionManager,
70+
DrmSessionEventListener.EventDispatcher drmEventDispatcher,
71+
Format initialFormat,
72+
List<FakeSampleStream.FakeSampleStreamItem> fakeSampleStreamItems) {
73+
return new FakeSampleStream(
74+
allocator,
75+
mediaSourceEventDispatcher,
76+
drmSessionManager,
77+
drmEventDispatcher,
78+
initialFormat,
79+
/* fakeSampleStreamItems= */ ImmutableList.of()) {
80+
private boolean addedSamples = false;
81+
82+
@Override
83+
public int readData(
84+
FormatHolder formatHolder, DecoderInputBuffer buffer, @ReadFlags int readFlags) {
85+
int result = super.readData(formatHolder, buffer, readFlags);
86+
if (!addedSamples) {
87+
append(
88+
ImmutableList.of(
89+
oneByteSample(/* timeUs= */ 0, C.BUFFER_FLAG_KEY_FRAME),
90+
oneByteSample(/* timeUs= */ 200, C.BUFFER_FLAG_KEY_FRAME),
91+
oneByteSample(/* timeUs= */ 400, C.BUFFER_FLAG_KEY_FRAME),
92+
oneByteSample(/* timeUs= */ 600, C.BUFFER_FLAG_KEY_FRAME),
93+
oneByteSample(/* timeUs= */ 800, C.BUFFER_FLAG_KEY_FRAME),
94+
END_OF_STREAM_ITEM));
95+
writeData(/* startPositionUs= */ 0);
96+
addedSamples = true;
97+
}
98+
return result;
99+
}
100+
};
101+
}
102+
};
103+
ClippingMediaPeriod clippingMediaPeriod =
104+
new ClippingMediaPeriod(
105+
mediaPeriod,
106+
/* enableInitialDiscontinuity= */ true,
107+
/* startUs= */ 0,
108+
/* endUs= */ 500);
109+
AtomicBoolean periodPrepared = new AtomicBoolean();
110+
clippingMediaPeriod.prepare(
111+
new MediaPeriod.Callback() {
112+
@Override
113+
public void onPrepared(MediaPeriod mediaPeriod) {
114+
periodPrepared.set(true);
115+
}
116+
117+
@Override
118+
public void onContinueLoadingRequested(MediaPeriod source) {
119+
clippingMediaPeriod.continueLoading(/* positionUs= */ 0);
120+
}
121+
},
122+
/* positionUs= */ 0);
123+
RobolectricUtil.runMainLooperUntil(periodPrepared::get);
124+
SampleStream[] sampleStreams = new SampleStream[1];
125+
clippingMediaPeriod.selectTracks(
126+
new ExoTrackSelection[] {new FixedTrackSelection(trackGroup, /* track= */ 0)},
127+
/* mayRetainStreamFlags= */ new boolean[] {false},
128+
sampleStreams,
129+
/* streamResetFlags= */ new boolean[] {true},
130+
/* positionUs= */ 0);
131+
FormatHolder formatHolder = new FormatHolder();
132+
DecoderInputBuffer buffer =
133+
new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_NORMAL);
134+
ArrayList<Long> readSamples = new ArrayList<>();
135+
136+
int result;
137+
do {
138+
result = sampleStreams[0].readData(formatHolder, buffer, /* readFlags= */ 0);
139+
if (result == C.RESULT_BUFFER_READ && !buffer.isEndOfStream()) {
140+
readSamples.add(buffer.timeUs);
141+
}
142+
} while (result != C.RESULT_BUFFER_READ || !buffer.isEndOfStream());
143+
144+
assertThat(readSamples).containsExactly(0L, 200L, 400L).inOrder();
145+
}
146+
}

‎libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ads/ServerSideAdInsertionMediaSourceTest.java

+143
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import static androidx.media3.common.C.DATA_TYPE_MEDIA;
1919
import static androidx.media3.common.util.Assertions.checkNotNull;
2020
import static androidx.media3.exoplayer.source.ads.ServerSideAdInsertionUtil.addAdGroupToAdPlaybackState;
21+
import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.END_OF_STREAM_ITEM;
22+
import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.oneByteSample;
2123
import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil;
2224
import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.playUntilPosition;
2325
import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilIsLoading;
@@ -45,29 +47,46 @@
4547
import androidx.media3.common.MediaItem;
4648
import androidx.media3.common.Player;
4749
import androidx.media3.common.Timeline;
50+
import androidx.media3.common.TrackGroup;
4851
import androidx.media3.common.util.Clock;
4952
import androidx.media3.common.util.Util;
53+
import androidx.media3.datasource.TransferListener;
54+
import androidx.media3.decoder.DecoderInputBuffer;
5055
import androidx.media3.exoplayer.ExoPlayer;
56+
import androidx.media3.exoplayer.FormatHolder;
5157
import androidx.media3.exoplayer.analytics.AnalyticsListener;
5258
import androidx.media3.exoplayer.analytics.PlayerId;
59+
import androidx.media3.exoplayer.drm.DrmSessionEventListener;
60+
import androidx.media3.exoplayer.drm.DrmSessionManager;
5361
import androidx.media3.exoplayer.source.DefaultMediaSourceFactory;
5462
import androidx.media3.exoplayer.source.MediaLoadData;
5563
import androidx.media3.exoplayer.source.MediaPeriod;
5664
import androidx.media3.exoplayer.source.MediaSource;
5765
import androidx.media3.exoplayer.source.MediaSourceEventListener;
66+
import androidx.media3.exoplayer.source.SampleStream;
5867
import androidx.media3.exoplayer.source.SinglePeriodTimeline;
68+
import androidx.media3.exoplayer.source.TrackGroupArray;
69+
import androidx.media3.exoplayer.trackselection.ExoTrackSelection;
70+
import androidx.media3.exoplayer.trackselection.FixedTrackSelection;
71+
import androidx.media3.exoplayer.upstream.Allocator;
5972
import androidx.media3.exoplayer.upstream.DefaultAllocator;
6073
import androidx.media3.test.utils.CapturingRenderersFactory;
6174
import androidx.media3.test.utils.DumpFileAsserts;
6275
import androidx.media3.test.utils.FakeClock;
76+
import androidx.media3.test.utils.FakeMediaPeriod;
6377
import androidx.media3.test.utils.FakeMediaSource;
78+
import androidx.media3.test.utils.FakeSampleStream;
6479
import androidx.media3.test.utils.FakeTimeline;
6580
import androidx.media3.test.utils.robolectric.PlaybackOutput;
81+
import androidx.media3.test.utils.robolectric.RobolectricUtil;
6682
import androidx.media3.test.utils.robolectric.ShadowMediaCodecConfig;
6783
import androidx.test.core.app.ApplicationProvider;
6884
import androidx.test.ext.junit.runners.AndroidJUnit4;
85+
import com.google.common.collect.ImmutableList;
6986
import com.google.common.collect.ImmutableMap;
7087
import java.util.ArrayList;
88+
import java.util.List;
89+
import java.util.concurrent.atomic.AtomicBoolean;
7190
import java.util.concurrent.atomic.AtomicReference;
7291
import org.junit.Assert;
7392
import org.junit.Rule;
@@ -689,4 +708,128 @@ public void playbackWithSeek_isHandledCorrectly() throws Exception {
689708
// Assert playback progression was smooth (=no unexpected delays that cause audio to underrun)
690709
verify(listener, never()).onAudioUnderrun(any(), anyInt(), anyLong(), anyLong());
691710
}
711+
712+
@Test
713+
public void serverSideAdInsertionSampleStream_withFastLoadingSourceAfterFirstRead_canBeReadFully()
714+
throws Exception {
715+
TrackGroup trackGroup = new TrackGroup(new Format.Builder().build());
716+
// Set up MediaPeriod with no samples and only add samples after the first SampleStream read.
717+
FakeMediaPeriod mediaPeriod =
718+
new FakeMediaPeriod(
719+
new TrackGroupArray(trackGroup),
720+
new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024),
721+
/* trackDataFactory= */ (format, mediaPeriodId) -> ImmutableList.of(),
722+
new MediaSourceEventListener.EventDispatcher()
723+
.withParameters(
724+
/* windowIndex= */ 0,
725+
new MediaSource.MediaPeriodId(/* periodUid= */ new Object())),
726+
DrmSessionManager.DRM_UNSUPPORTED,
727+
new DrmSessionEventListener.EventDispatcher(),
728+
/* deferOnPrepared= */ false) {
729+
@Override
730+
protected FakeSampleStream createSampleStream(
731+
Allocator allocator,
732+
@Nullable MediaSourceEventListener.EventDispatcher mediaSourceEventDispatcher,
733+
DrmSessionManager drmSessionManager,
734+
DrmSessionEventListener.EventDispatcher drmEventDispatcher,
735+
Format initialFormat,
736+
List<FakeSampleStream.FakeSampleStreamItem> fakeSampleStreamItems) {
737+
return new FakeSampleStream(
738+
allocator,
739+
mediaSourceEventDispatcher,
740+
drmSessionManager,
741+
drmEventDispatcher,
742+
initialFormat,
743+
/* fakeSampleStreamItems= */ ImmutableList.of()) {
744+
private boolean addedSamples = false;
745+
746+
@Override
747+
public int readData(
748+
FormatHolder formatHolder, DecoderInputBuffer buffer, @ReadFlags int readFlags) {
749+
int result = super.readData(formatHolder, buffer, readFlags);
750+
if (!addedSamples) {
751+
append(
752+
ImmutableList.of(
753+
oneByteSample(/* timeUs= */ 0, C.BUFFER_FLAG_KEY_FRAME),
754+
oneByteSample(/* timeUs= */ 200, C.BUFFER_FLAG_KEY_FRAME),
755+
oneByteSample(/* timeUs= */ 400, C.BUFFER_FLAG_KEY_FRAME),
756+
oneByteSample(/* timeUs= */ 600, C.BUFFER_FLAG_KEY_FRAME),
757+
oneByteSample(/* timeUs= */ 800, C.BUFFER_FLAG_KEY_FRAME),
758+
END_OF_STREAM_ITEM));
759+
writeData(/* startPositionUs= */ 0);
760+
addedSamples = true;
761+
}
762+
return result;
763+
}
764+
};
765+
}
766+
};
767+
FakeMediaSource mediaSource =
768+
new FakeMediaSource() {
769+
@Override
770+
protected MediaPeriod createMediaPeriod(
771+
MediaPeriodId id,
772+
TrackGroupArray trackGroupArray,
773+
Allocator allocator,
774+
MediaSourceEventListener.EventDispatcher mediaSourceEventDispatcher,
775+
DrmSessionManager drmSessionManager,
776+
DrmSessionEventListener.EventDispatcher drmEventDispatcher,
777+
@Nullable TransferListener transferListener) {
778+
return mediaPeriod;
779+
}
780+
};
781+
ServerSideAdInsertionMediaSource serverSideAdInsertionMediaSource =
782+
new ServerSideAdInsertionMediaSource(mediaSource, /* adPlaybackStateUpdater= */ null);
783+
Timeline timeline = new FakeTimeline();
784+
Object periodUid = timeline.getUidOfPeriod(/* periodIndex= */ 0);
785+
serverSideAdInsertionMediaSource.setAdPlaybackStates(
786+
ImmutableMap.of(periodUid, new AdPlaybackState(/* adsId= */ new Object())), timeline);
787+
AtomicBoolean sourcePrepared = new AtomicBoolean();
788+
serverSideAdInsertionMediaSource.prepareSource(
789+
(source, newTimeline) -> sourcePrepared.set(true),
790+
/* mediaTransferListener= */ null,
791+
PlayerId.UNSET);
792+
RobolectricUtil.runMainLooperUntil(sourcePrepared::get);
793+
MediaPeriod serverSideAdInsertionMediaPeriod =
794+
serverSideAdInsertionMediaSource.createPeriod(
795+
new MediaSource.MediaPeriodId(periodUid),
796+
/* allocator= */ null,
797+
/* startPositionUs= */ 0);
798+
AtomicBoolean periodPrepared = new AtomicBoolean();
799+
serverSideAdInsertionMediaPeriod.prepare(
800+
new MediaPeriod.Callback() {
801+
@Override
802+
public void onPrepared(MediaPeriod mediaPeriod) {
803+
periodPrepared.set(true);
804+
}
805+
806+
@Override
807+
public void onContinueLoadingRequested(MediaPeriod source) {
808+
serverSideAdInsertionMediaPeriod.continueLoading(/* positionUs= */ 0);
809+
}
810+
},
811+
/* positionUs= */ 0);
812+
RobolectricUtil.runMainLooperUntil(periodPrepared::get);
813+
SampleStream[] sampleStreams = new SampleStream[1];
814+
serverSideAdInsertionMediaPeriod.selectTracks(
815+
new ExoTrackSelection[] {new FixedTrackSelection(trackGroup, /* track= */ 0)},
816+
/* mayRetainStreamFlags= */ new boolean[] {false},
817+
sampleStreams,
818+
/* streamResetFlags= */ new boolean[] {true},
819+
/* positionUs= */ 0);
820+
FormatHolder formatHolder = new FormatHolder();
821+
DecoderInputBuffer buffer =
822+
new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_NORMAL);
823+
ArrayList<Long> readSamples = new ArrayList<>();
824+
825+
int result;
826+
do {
827+
result = sampleStreams[0].readData(formatHolder, buffer, /* readFlags= */ 0);
828+
if (result == C.RESULT_BUFFER_READ && !buffer.isEndOfStream()) {
829+
readSamples.add(buffer.timeUs);
830+
}
831+
} while (result != C.RESULT_BUFFER_READ || !buffer.isEndOfStream());
832+
833+
assertThat(readSamples).containsExactly(0L, 200L, 400L, 600L, 800L).inOrder();
834+
}
692835
}

0 commit comments

Comments
 (0)