Skip to content

Commit 218edf0

Browse files
clainclycopybara-github
authored andcommitted
Release the players before creating new PlaybackAudioGraphWrapper
Previously in `setComposition()`, a new `PlaybackAudioGraphWrapper` is created (and old `PlaybackAudioGraphWrapper` released) before releasing the players. This causes race conditions when the player accesses the already released `PlaybackAudioGraphWrapper` before it's released. PiperOrigin-RevId: 791856930
1 parent d910b67 commit 218edf0

File tree

1 file changed

+27
-16
lines changed

1 file changed

+27
-16
lines changed

libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@ public CompositionPlayer build() {
437437
private LivePositionSupplier totalBufferedDurationSupplier;
438438
private boolean compositionPlayerInternalPrepared;
439439
private boolean scrubbingModeEnabled;
440+
// Whether prepare() needs to be called to prepare the underlying sequence players.
441+
// TODO: b/436491202 - Revise CompositionPlayer state handling.
442+
private boolean appNeedsToPrepareCompositionPlayer;
440443

441444
// "this" reference for position suppliers.
442445
@SuppressWarnings("initialization:methodref.receiver.bound.invalid")
@@ -462,6 +465,7 @@ private CompositionPlayer(Builder builder) {
462465
positionSupplier = new LivePositionSupplier(this::getContentPositionMs);
463466
bufferedPositionSupplier = new LivePositionSupplier(this::getBufferedPositionMs);
464467
totalBufferedDurationSupplier = new LivePositionSupplier(this::getTotalBufferedDurationMs);
468+
appNeedsToPrepareCompositionPlayer = true;
465469
}
466470

467471
/**
@@ -664,6 +668,7 @@ protected ListenableFuture<?> handlePrepare() {
664668
for (int i = 0; i < playerHolders.size(); i++) {
665669
playerHolders.get(i).player.prepare();
666670
}
671+
appNeedsToPrepareCompositionPlayer = false;
667672
return Futures.immediateVoidFuture();
668673
}
669674

@@ -697,6 +702,7 @@ protected ListenableFuture<?> handleStop() {
697702
for (int i = 0; i < playerHolders.size(); i++) {
698703
playerHolders.get(i).player.stop();
699704
}
705+
appNeedsToPrepareCompositionPlayer = true;
700706
return Futures.immediateVoidFuture();
701707
}
702708

@@ -924,22 +930,39 @@ private void prepareCompositionPlayerInternal() {
924930
}
925931

926932
private void setCompositionInternal(Composition composition, long startPositionMs) {
933+
for (int i = 0; i < playerHolders.size(); i++) {
934+
// TODO: b/412585856 - Optimize for the case where we can keep some resources.
935+
playerHolders.get(i).player.release();
936+
}
937+
playerHolders.clear();
938+
927939
prepareCompositionPlayerInternal();
928940
CompositionPlayerInternal compositionPlayerInternal =
929941
checkNotNull(this.compositionPlayerInternal);
930942
compositionDurationUs = getCompositionDurationUs(composition);
931943
long primarySequenceDurationUs =
932944
getSequenceDurationUs(checkNotNull(composition.sequences.get(0)));
933945
for (int i = 0; i < composition.sequences.size(); i++) {
934-
setSequenceInternal(
946+
createSequencePlayer(
935947
composition, /* sequenceIndex= */ i, primarySequenceDurationUs, startPositionMs);
936948
}
937949
compositionPlayerInternal.setComposition(composition);
938950
compositionPlayerInternal.startSeek(startPositionMs);
939951
compositionPlayerInternal.endSeek();
952+
953+
if (appNeedsToPrepareCompositionPlayer) {
954+
return;
955+
}
956+
// After the app calls prepare() for the first time, subsequent calls to setComposition()
957+
// doesn't require apps to call prepare() again (unless stop() is called, or player error
958+
// reported), hence the need to prepare the underlying players here.
959+
for (int i = 0; i < playerHolders.size(); i++) {
960+
SequencePlayerHolder sequencePlayerHolder = playerHolders.get(i);
961+
sequencePlayerHolder.player.prepare();
962+
}
940963
}
941964

942-
private void setSequenceInternal(
965+
private void createSequencePlayer(
943966
Composition newComposition,
944967
int sequenceIndex,
945968
long primarySequenceDurationUs,
@@ -951,15 +974,7 @@ private void setSequenceInternal(
951974
createSequencePlayer(
952975
sequenceIndex,
953976
newComposition.hdrMode == Composition.HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_MEDIACODEC);
954-
if (playerHolders.size() <= sequenceIndex) {
955-
playerHolders.add(playerHolder);
956-
} else {
957-
// TODO: b/412585856 - Optimize for the case where we can keep some resources.
958-
playerHolders.get(sequenceIndex).player.release();
959-
// TODO: b/412585856 - Remove dangling playerHolder references after changing the number of
960-
// sequences.
961-
playerHolders.set(sequenceIndex, playerHolder);
962-
}
977+
playerHolders.add(playerHolder);
963978
playerHolder.setSequence(sequence);
964979
ExoPlayer player = playerHolder.player;
965980

@@ -988,11 +1003,6 @@ private void setSequenceInternal(
9881003
invalidateState();
9891004
playlist = createPlaylist();
9901005
}
991-
992-
if (playbackState != STATE_IDLE) {
993-
player.stop();
994-
player.prepare();
995-
}
9961006
}
9971007

9981008
private SequencePlayerHolder createSequencePlayer(
@@ -1245,6 +1255,7 @@ private void maybeUpdatePlaybackError(
12451255
for (int i = 0; i < playerHolders.size(); i++) {
12461256
playerHolders.get(i).player.stop();
12471257
}
1258+
appNeedsToPrepareCompositionPlayer = true;
12481259
updatePlaybackState();
12491260
// Invalidate the parent class state.
12501261
invalidateState();

0 commit comments

Comments
 (0)