RtspMediaPeriod: Use a new ExtractorOutput for each SampleQueue

This removes concurrent access from `rtspLoaderWrappers`. Previously
there was a race between the playback thread clearing & re-adding
entries to this list in `retryWithRtpTcp()`, and the loading thread
accessing the entries in `InternalListener.track()` (implemented from
`ExtractorOutput`).

This change means each `ExtractorOutputImpl` uses exactly one
`SampleQueue` for its one `TrackOutput`. When the `RtspLoaderWrapper`
instances are replaced in `retryWithRtpTcp()`, any stale instances will
only be able to access their own (also stale) `SampleQueue` instances
(vs before, where the stale `ExtractorOutput` could accidentally access
'new' `SampleQueue` instances via the `rtspLoaderWrappers` field).

As well as fixing a race condition in the prod code, this also de-flakes
`RtspPlaybackTest`.

#minor-release

PiperOrigin-RevId: 559130479
This commit is contained in:
ibaker 2023-08-22 17:11:46 +01:00 committed by Ian Baker
parent 398809e4e2
commit 9533f5cd1c
2 changed files with 23 additions and 11 deletions

View File

@ -36,6 +36,8 @@
* HLS Extension:
* Smooth Streaming Extension:
* RTSP Extension:
* Fix a race condition that could lead to `IndexOutOfBoundsException` when
falling back to TCP, or playback hanging in some situations.
* Decoder Extensions (FFmpeg, VP9, AV1, etc.):
* MIDI extension:
* Cast Extension:

View File

@ -500,18 +500,18 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
return listBuilder.build();
}
private final class InternalListener
implements ExtractorOutput,
Loader.Callback<RtpDataLoadable>,
UpstreamFormatChangedListener,
SessionInfoListener,
PlaybackEventListener {
// All interactions are on the loading thread
private final class ExtractorOutputImpl implements ExtractorOutput {
// ExtractorOutput implementation.
private final TrackOutput trackOutput;
private ExtractorOutputImpl(TrackOutput trackOutput) {
this.trackOutput = trackOutput;
}
@Override
public TrackOutput track(int id, int type) {
return checkNotNull(rtspLoaderWrappers.get(id)).sampleQueue;
return trackOutput;
}
@Override
@ -523,6 +523,13 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
public void seekMap(SeekMap seekMap) {
// RTSP does not support seek map.
}
}
private final class InternalListener
implements Loader.Callback<RtpDataLoadable>,
UpstreamFormatChangedListener,
SessionInfoListener,
PlaybackEventListener {
// Loadable.Callback implementation.
@ -790,9 +797,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
*/
public RtspLoaderWrapper(
RtspMediaTrack mediaTrack, int trackId, RtpDataChannel.Factory rtpDataChannelFactory) {
loadInfo = new RtpLoadInfo(mediaTrack, trackId, rtpDataChannelFactory);
loader = new Loader("ExoPlayer:RtspMediaPeriod:RtspLoaderWrapper " + trackId);
sampleQueue = SampleQueue.createWithoutDrm(allocator);
loadInfo = new RtpLoadInfo(mediaTrack, trackId, sampleQueue, rtpDataChannelFactory);
sampleQueue.setUpstreamFormatChangeListener(internalListener);
}
@ -875,7 +882,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
/** Creates a new instance. */
public RtpLoadInfo(
RtspMediaTrack mediaTrack, int trackId, RtpDataChannel.Factory rtpDataChannelFactory) {
RtspMediaTrack mediaTrack,
int trackId,
TrackOutput trackOutput,
RtpDataChannel.Factory rtpDataChannelFactory) {
this.mediaTrack = mediaTrack;
// This listener runs on the playback thread, posted by the Loader thread.
@ -899,7 +909,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
trackId,
mediaTrack,
/* eventListener= */ transportEventListener,
/* output= */ internalListener,
/* output= */ new ExtractorOutputImpl(trackOutput),
rtpDataChannelFactory);
}