From 9533f5cd1c8480409646d46da3cf3145b8c7b33d Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Tue, 22 Aug 2023 17:11:46 +0100
Subject: [PATCH] `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
---
 RELEASENOTES.md                               |  2 ++
 .../exoplayer/rtsp/RtspMediaPeriod.java       | 32 ++++++++++++-------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/RELEASENOTES.md b/RELEASENOTES.md
index bdc25b4bc4..4c788ab74f 100644
--- a/RELEASENOTES.md
+++ b/RELEASENOTES.md
@@ -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:
diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaPeriod.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaPeriod.java
index a8c8e6fe40..43769007b1 100644
--- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaPeriod.java
+++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaPeriod.java
@@ -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);
     }