Commit 85bc1d6e by tonihei Committed by Oliver Woodman

Fix message indexing bug.

We keep an index hint for the next pending player message. This hint
wasn't updated correctly when messages are removed due to a timeline
update.

This change makes sure to only use the hint locally in one method so
that it doesn't need to be updated anywhere else and also adds the "hint"
suffix to the variable name to make it clearer that it's just a hint and
there are no guarantees this index actually exists anymore.

issue:#7278
PiperOrigin-RevId: 309217614
parent 19509059
...@@ -6,6 +6,9 @@ ...@@ -6,6 +6,9 @@
* Add opt-in to verify correct thread usage with * Add opt-in to verify correct thread usage with
`SimpleExoPlayer.setThrowsWhenUsingWrongThread(true)` `SimpleExoPlayer.setThrowsWhenUsingWrongThread(true)`
([#4463](https://github.com/google/ExoPlayer/issues/4463)). ([#4463](https://github.com/google/ExoPlayer/issues/4463)).
* Fix bug where `PlayerMessages` throw an exception after `MediaSources`
are removed from the playlist
([#7278](https://github.com/google/ExoPlayer/issues/7278)).
* Add playbackPositionUs parameter to 'LoadControl.shouldContinueLoading'. * Add playbackPositionUs parameter to 'LoadControl.shouldContinueLoading'.
* The `DefaultLoadControl` default minimum buffer is set to 50 seconds, * The `DefaultLoadControl` default minimum buffer is set to 50 seconds,
equal to the default maximum buffer. `DefaultLoadControl` applies the equal to the default maximum buffer. `DefaultLoadControl` applies the
...@@ -107,10 +110,6 @@ ...@@ -107,10 +110,6 @@
* Implement timing-out of stuck CEA-608 captions (as permitted by * Implement timing-out of stuck CEA-608 captions (as permitted by
ANSI/CTA-608-E R-2014 Annex C.9) and set the default timeout to 16 ANSI/CTA-608-E R-2014 Annex C.9) and set the default timeout to 16
seconds ([#7181](https://github.com/google/ExoPlayer/issues/7181)). seconds ([#7181](https://github.com/google/ExoPlayer/issues/7181)).
* Add special-case positioning behaviour for vertical cues being rendered
horizontally.
* Implement steps 4-10 of the
[WebVTT line computation algorithm](https://www.w3.org/TR/webvtt1/#cue-computed-line).
* DRM: * DRM:
* Add support for attaching DRM sessions to clear content in the demo app. * Add support for attaching DRM sessions to clear content in the demo app.
* Remove `DrmSessionManager` references from all renderers. * Remove `DrmSessionManager` references from all renderers.
...@@ -151,8 +150,6 @@ ...@@ -151,8 +150,6 @@
* Upgrade Truth dependency from 0.44 to 1.0. * Upgrade Truth dependency from 0.44 to 1.0.
* Upgrade to JUnit 4.13-rc-2. * Upgrade to JUnit 4.13-rc-2.
* UI * UI
* Remove deperecated `exo_simpe_player_view.xml` and
`exo_playback_control_view.xml` from resource.
* Add `showScrubber` and `hideScrubber` methods to DefaultTimeBar. * Add `showScrubber` and `hideScrubber` methods to DefaultTimeBar.
* Move logic of prev, next, fast forward and rewind to ControlDispatcher * Move logic of prev, next, fast forward and rewind to ControlDispatcher
([#6926](https://github.com/google/ExoPlayer/issues/6926)). ([#6926](https://github.com/google/ExoPlayer/issues/6926)).
......
...@@ -131,7 +131,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -131,7 +131,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
private int enabledRendererCount; private int enabledRendererCount;
@Nullable private SeekPosition pendingInitialSeekPosition; @Nullable private SeekPosition pendingInitialSeekPosition;
private long rendererPositionUs; private long rendererPositionUs;
private int nextPendingMessageIndex; private int nextPendingMessageIndexHint;
private boolean deliverPendingMessageAtStartPositionRequired; private boolean deliverPendingMessageAtStartPositionRequired;
private long releaseTimeoutMs; private long releaseTimeoutMs;
...@@ -1191,7 +1191,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1191,7 +1191,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
pendingMessageInfo.message.markAsProcessed(/* isDelivered= */ false); pendingMessageInfo.message.markAsProcessed(/* isDelivered= */ false);
} }
pendingMessages.clear(); pendingMessages.clear();
nextPendingMessageIndex = 0;
resetPosition = true; resetPosition = true;
} }
MediaPeriodId mediaPeriodId = playbackInfo.periodId; MediaPeriodId mediaPeriodId = playbackInfo.periodId;
...@@ -1365,6 +1364,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1365,6 +1364,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
// Correct next index if necessary (e.g. after seeking, timeline changes, or new messages) // Correct next index if necessary (e.g. after seeking, timeline changes, or new messages)
int currentPeriodIndex = int currentPeriodIndex =
playbackInfo.timeline.getIndexOfPeriod(playbackInfo.periodId.periodUid); playbackInfo.timeline.getIndexOfPeriod(playbackInfo.periodId.periodUid);
int nextPendingMessageIndex = Math.min(nextPendingMessageIndexHint, pendingMessages.size());
PendingMessageInfo previousInfo = PendingMessageInfo previousInfo =
nextPendingMessageIndex > 0 ? pendingMessages.get(nextPendingMessageIndex - 1) : null; nextPendingMessageIndex > 0 ? pendingMessages.get(nextPendingMessageIndex - 1) : null;
while (previousInfo != null while (previousInfo != null
...@@ -1410,6 +1410,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1410,6 +1410,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
? pendingMessages.get(nextPendingMessageIndex) ? pendingMessages.get(nextPendingMessageIndex)
: null; : null;
} }
nextPendingMessageIndexHint = nextPendingMessageIndex;
} }
private void ensureStopped(Renderer renderer) throws ExoPlaybackException { private void ensureStopped(Renderer renderer) throws ExoPlaybackException {
......
...@@ -19,6 +19,11 @@ import static com.google.common.truth.Truth.assertThat; ...@@ -19,6 +19,11 @@ import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.robolectric.Shadows.shadowOf; import static org.robolectric.Shadows.shadowOf;
import android.content.Context; import android.content.Context;
...@@ -2314,6 +2319,43 @@ public final class ExoPlayerTest { ...@@ -2314,6 +2319,43 @@ public final class ExoPlayerTest {
} }
@Test @Test
public void sendMessages_withMediaRemoval_triggersCorrectMessagesAndDoesNotThrow()
throws Exception {
ExoPlayer player = new TestExoPlayer.Builder(context).build();
MediaSource mediaSource = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1));
player.addMediaSources(Arrays.asList(mediaSource, mediaSource));
player
.createMessage((messageType, payload) -> {})
.setPosition(/* windowIndex= */ 0, /* positionMs= */ 0)
.setDeleteAfterDelivery(false)
.send();
PlayerMessage.Target secondMediaItemTarget = mock(PlayerMessage.Target.class);
player
.createMessage(secondMediaItemTarget)
.setPosition(/* windowIndex= */ 1, /* positionMs= */ 0)
.setDeleteAfterDelivery(false)
.send();
// Play through media once to trigger all messages. This ensures any internally saved message
// indices are non-zero.
player.prepare();
player.play();
TestExoPlayer.runUntilPlaybackState(player, Player.STATE_ENDED);
verify(secondMediaItemTarget).handleMessage(anyInt(), any());
// Remove first item and play second item again to check if message is triggered again.
// After removal, any internally saved message indices are invalid and will throw
// IndexOutOfBoundsException if used without updating.
// See https://github.com/google/ExoPlayer/issues/7278.
player.removeMediaItem(/* index= */ 0);
player.seekTo(/* positionMs= */ 0);
TestExoPlayer.runUntilPlaybackState(player, Player.STATE_ENDED);
assertThat(player.getPlayerError()).isNull();
verify(secondMediaItemTarget, times(2)).handleMessage(anyInt(), any());
}
@Test
public void setAndSwitchSurface() throws Exception { public void setAndSwitchSurface() throws Exception {
final List<Integer> rendererMessages = new ArrayList<>(); final List<Integer> rendererMessages = new ArrayList<>();
Renderer videoRenderer = Renderer videoRenderer =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment