Skip to content

Commit b792b52

Browse files
nift4nift4
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent bb105d3 commit b792b52

File tree

3 files changed

+131
-96
lines changed

3 files changed

+131
-96
lines changed

‎libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java

Lines changed: 106 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,20 +1080,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
10801080
});
10811081
}
10821082

1083-
/* package */ boolean onPlayRequested() {
1083+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
10841084
if (Looper.myLooper() != Looper.getMainLooper()) {
10851085
SettableFuture<Boolean> playRequested = SettableFuture.create();
1086-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
1087-
try {
1088-
return playRequested.get();
1089-
} catch (InterruptedException | ExecutionException e) {
1090-
throw new IllegalStateException(e);
1091-
}
1086+
mainHandler.post(
1087+
() -> {
1088+
try {
1089+
playRequested.set(onPlayRequested().get());
1090+
} catch (ExecutionException | InterruptedException e) {
1091+
playRequested.setException(new IllegalStateException(e));
1092+
}
1093+
});
1094+
return playRequested;
10921095
}
10931096
if (this.mediaSessionListener != null) {
1094-
return this.mediaSessionListener.onPlayRequested(instance);
1097+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
10951098
}
1096-
return true;
1099+
return Futures.immediateFuture(true);
10971100
}
10981101

10991102
/**
@@ -1104,84 +1107,103 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
11041107
*
11051108
* @param controller The controller requesting to play.
11061109
*/
1107-
/* package */ void handleMediaControllerPlayRequest(
1110+
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
11081111
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
1109-
if (!onPlayRequested()) {
1110-
// Request denied, e.g. due to missing foreground service abilities.
1111-
return;
1112-
}
1113-
boolean hasCurrentMediaItem =
1114-
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1115-
&& playerWrapper.getCurrentMediaItem() != null;
1116-
boolean canAddMediaItems =
1117-
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1118-
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1119-
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1120-
Player.Commands playCommand =
1121-
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1122-
if (hasCurrentMediaItem || !canAddMediaItems) {
1123-
// No playback resumption needed or possible.
1124-
if (!hasCurrentMediaItem) {
1125-
Log.w(
1126-
TAG,
1127-
"Play requested without current MediaItem, but playback resumption prevented by"
1128-
+ " missing available commands");
1129-
}
1130-
Util.handlePlayButtonAction(playerWrapper);
1131-
if (callOnPlayerInteractionFinished) {
1132-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1133-
}
1134-
} else {
1135-
@Nullable
1136-
ListenableFuture<MediaItemsWithStartPosition> future =
1137-
checkNotNull(
1138-
callback.onPlaybackResumption(
1139-
instance, controllerForRequest, /* isForPlayback= */ true),
1140-
"Callback.onPlaybackResumption must return a non-null future");
1141-
Futures.addCallback(
1142-
future,
1143-
new FutureCallback<MediaItemsWithStartPosition>() {
1144-
@Override
1145-
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1146-
callWithControllerForCurrentRequestSet(
1147-
controllerForRequest,
1148-
() -> {
1149-
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1150-
playerWrapper, mediaItemsWithStartPosition);
1151-
Util.handlePlayButtonAction(playerWrapper);
1152-
if (callOnPlayerInteractionFinished) {
1153-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1154-
}
1155-
})
1156-
.run();
1112+
SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
1113+
ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
1114+
playRequestedFuture.addListener(
1115+
() -> {
1116+
boolean playRequested;
1117+
try {
1118+
playRequested = playRequestedFuture.get();
1119+
} catch (ExecutionException | InterruptedException e) {
1120+
sessionFuture.setException(new IllegalStateException(e));
1121+
return;
1122+
}
1123+
if (!playRequested) {
1124+
// Request denied, e.g. due to missing foreground service abilities.
1125+
sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
1126+
return;
1127+
}
1128+
boolean hasCurrentMediaItem =
1129+
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1130+
&& playerWrapper.getCurrentMediaItem() != null;
1131+
boolean canAddMediaItems =
1132+
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1133+
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1134+
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1135+
Player.Commands playCommand =
1136+
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1137+
if (hasCurrentMediaItem || !canAddMediaItems) {
1138+
// No playback resumption needed or possible.
1139+
if (!hasCurrentMediaItem) {
1140+
Log.w(
1141+
TAG,
1142+
"Play requested without current MediaItem, but playback resumption prevented by"
1143+
+ " missing available commands");
11571144
}
1158-
1159-
@Override
1160-
public void onFailure(Throwable t) {
1161-
if (t instanceof UnsupportedOperationException) {
1162-
Log.w(
1163-
TAG,
1164-
"UnsupportedOperationException: Make sure to implement"
1165-
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
1166-
+ " media button receiver to your manifest or if you implement the recent"
1167-
+ " media item contract with your MediaLibraryService.",
1168-
t);
1169-
} else {
1170-
Log.e(
1171-
TAG,
1172-
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1173-
+ t.getMessage(),
1174-
t);
1175-
}
1176-
// Play as requested even if playback resumption fails.
1177-
Util.handlePlayButtonAction(playerWrapper);
1178-
if (callOnPlayerInteractionFinished) {
1179-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1180-
}
1145+
Util.handlePlayButtonAction(playerWrapper);
1146+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1147+
if (callOnPlayerInteractionFinished) {
1148+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
11811149
}
1182-
},
1183-
this::postOrRunOnApplicationHandler);
1184-
}
1150+
} else {
1151+
@Nullable
1152+
ListenableFuture<MediaItemsWithStartPosition> future =
1153+
checkNotNull(
1154+
callback.onPlaybackResumption(
1155+
instance, controllerForRequest, /* isForPlayback= */ true),
1156+
"Callback.onPlaybackResumption must return a non-null future");
1157+
Futures.addCallback(
1158+
future,
1159+
new FutureCallback<MediaItemsWithStartPosition>() {
1160+
@Override
1161+
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1162+
callWithControllerForCurrentRequestSet(
1163+
controllerForRequest,
1164+
() -> {
1165+
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1166+
playerWrapper, mediaItemsWithStartPosition);
1167+
Util.handlePlayButtonAction(playerWrapper);
1168+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1169+
if (callOnPlayerInteractionFinished) {
1170+
onPlayerInteractionFinishedOnHandler(
1171+
controllerForRequest, playCommand);
1172+
}
1173+
})
1174+
.run();
1175+
}
1176+
1177+
@Override
1178+
public void onFailure(Throwable t) {
1179+
if (t instanceof UnsupportedOperationException) {
1180+
Log.w(
1181+
TAG,
1182+
"UnsupportedOperationException: Make sure to implement"
1183+
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
1184+
+ " button receiver to your manifest or if you implement the recent"
1185+
+ " media item contract with your MediaLibraryService.",
1186+
t);
1187+
} else {
1188+
Log.e(
1189+
TAG,
1190+
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1191+
+ t.getMessage(),
1192+
t);
1193+
}
1194+
// Play as requested even if playback resumption fails.
1195+
Util.handlePlayButtonAction(playerWrapper);
1196+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1197+
if (callOnPlayerInteractionFinished) {
1198+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1199+
}
1200+
}
1201+
},
1202+
this::postOrRunOnApplicationHandler);
1203+
}
1204+
},
1205+
this::postOrRunOnApplicationHandler);
1206+
return sessionFuture;
11851207
}
11861208

11871209
/* package */ void triggerPlayerInfoUpdate() {

‎libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -601,9 +601,27 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) {
601601
public void onPlay() {
602602
dispatchSessionTaskWithPlayerCommand(
603603
COMMAND_PLAY_PAUSE,
604-
controller ->
605-
sessionImpl.handleMediaControllerPlayRequest(
606-
controller, /* callOnPlayerInteractionFinished= */ true),
604+
controller -> {
605+
ListenableFuture<SessionResult> resultFuture =
606+
sessionImpl.handleMediaControllerPlayRequest(
607+
controller, /* callOnPlayerInteractionFinished= */ true);
608+
Futures.addCallback(
609+
resultFuture,
610+
new FutureCallback<SessionResult>() {
611+
@Override
612+
public void onSuccess(SessionResult result) {
613+
if (result.resultCode != RESULT_SUCCESS) {
614+
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
615+
}
616+
}
617+
618+
@Override
619+
public void onFailure(Throwable t) {
620+
Log.e(TAG, "Unexpected exception in onPlay() of " + controller, t);
621+
}
622+
},
623+
MoreExecutors.directExecutor());
624+
},
607625
sessionCompat.getCurrentControllerInfo(),
608626
/* callOnPlayerInteractionFinished= */ false);
609627
}

‎libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -777,15 +777,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
777777
controller,
778778
sequenceNumber,
779779
COMMAND_PLAY_PAUSE,
780-
sendSessionResultSuccess(
781-
player -> {
782-
@Nullable MediaSessionImpl impl = sessionImpl.get();
783-
if (impl == null || impl.isReleased()) {
784-
return;
785-
}
786-
impl.handleMediaControllerPlayRequest(
787-
controller, /* callOnPlayerInteractionFinished= */ false);
788-
}));
780+
sendSessionResultWhenReady(
781+
(session, theController, sequenceId) ->
782+
session.handleMediaControllerPlayRequest(
783+
theController, /* callOnPlayerInteractionFinished= */ false)));
789784
}
790785

791786
@Override

0 commit comments

Comments
 (0)