How to support reordering during the shuffle #9889

mag0716 · 2022-01-24T08:12:55Z

Things I want to do

The app shows currently playing items
The app allows reordering during the shuffle
The result of reordering during the shuffle does not affect the song order before the shuffle

Current approach

Get currently playing items from MediaControllerCompat.Callback#onQueueChanged
Customize TimelineQueueEditor, ShuffleOrder
- doesn't call Player#moveMediaItem when reorder during the shuffle
- generate a new ShuffleOrder from the sorted result
- call ExoPlayer#setShuffleOrder to update the sort order

Result

When reordering several times while shuffling, the song order may not be reflected in MediaSession
doesn't call MediaControllerCompat.Callback#onQueueChanged

Environment

ExoPlayer:2.16.1
Test Device:
- Pixel 3(OS 12)
- Pixel 4(OS 11)
- Emulator(OS 10)
- Sample Project

Sample Project

https://github.com/mag0716/SamplePlayer

Question

Is it correct behavior that sometimes MediaControllerCompat.Callback#onQueueChanged is not executed when ExoPlayer#setShuffleOrder is called?
Is there a better approach to support reordering during the shuffle?

The text was updated successfully, but these errors were encountered:

marcbaechinger · 2022-01-24T16:07:06Z

Is it correct behavior that sometimes MediaControllerCompat.Callback#onQueueChanged is not executed when ExoPlayer#setShuffleOrder is called?

I would expect that setting a different shuffle order changes the timeline which triggers a queue update in the session.

So if the ShuffleOrder that you set is different to what has been set before, it should trigger a queue update.

Does it help if you call mediaSessionConnector.invalidateMediaSessionQueue() after you have set a new shuffle order? If that helps we would have to investigate why the timeline change is not happening.

Is there a better approach to support reordering during the shuffle?

I'm not sure whether I understand what you mean with reordering during the shuffle.

The correct approach is setting a new ShuffleOrder and not moving media items within the playlist. When the shuffle order changes, the media items still are at the same position in the playlist, but the sequence of indices returned by player.getNextMediaItemIndex()` is affected by the shuffle order. So I would expect that 'reordering' is not required if I understand correctly.

mag0716 · 2022-01-26T02:00:01Z

@marcbaechinger
Thanks for the reply.

I'm not sure whether I understand what you mean with reordering during the shuffle.

sorry.
reordering during the shuffle means that to reorder the songs when shuffle mode is enabled.

Does it help if you call mediaSessionConnector.invalidateMediaSessionQueue() after you have set a new shuffle order? If that helps we would have to investigate why the timeline change is not happening.
The correct approach is setting a new ShuffleOrder and not moving media items within the playlist. When the shuffle order changes, the media items still are at the same position in the playlist, but the sequence of indices returned by player.getNextMediaItemIndex()` is affected by the shuffle order. So I would expect that 'reordering' is not required if I understand correctly.

I will try to call mediaSessionConnector.invalidateMediaSessionQueue() then comment on the results.

mag0716 · 2022-01-26T08:50:15Z

@marcbaechinger
I try to call mediaSessionConnector.invalidateMediaSessionQueue() after set a new ShuffleOrder.
Sample code is here.

Results

MediaControllerCompat.Callback#onQueueChanged is called every time
But, the song order given in onQueueChanged is different from the song order in a new ShuffleOrder

2022-01-26 15:19:06.810 11702-11702/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media0, media6, media4, media1, media3, media7, media8, media5] <- enable shuffle mode
2022-01-26 15:19:07.100 11702-11702/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media0, media6, media4, media1, media3, media7, media8, media5]
2022-01-26 15:19:07.133 11702-11702/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media0, media6, media4, media1, media3, media7, media8, media5]
2022-01-26 15:19:20.627 11702-11702/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [2, 0, 6, 4, 1, 3, 7, 5, 8]                            <- move media5 previous to media8
2022-01-26 15:19:20.631 11702-11702/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media0, media6, media4, media1, media3, media7, media8, media5] <- onQueueChanged is called, but not in the correct song order

Is the place to call mediaSessionConnector.invalidateMediaSessionQueue() wrong?

marcbaechinger · 2022-01-30T21:18:48Z

You correctly do

player.setShuffleOrder(shuffleOrder);
mediaSessionConnector.invalidateMediaSessionQueue()

It looks to me like you are hitting a bug with the masked timeline. With the above code the MediaSessionConnector invalidates the queue which triggers queueNavigator.onTimelineChanged(player). The TimelineQueueNavigator then calls player.getCurrentTimeline() and receives the masked timeline that is created just at the beginning of ExoPlayerImpl.setShuffleOrder(shuffleOrder). The timeline created at that moment does still include the old shuffleOrder and not the one that is passed to the setter.

Looks like that we need to assign the new shuffleOrder before we create the masking timeline.

If this is true what I'm saying, you should receive the new shuffle order later. When the status update arrives from the playback thread.

This should possibly already do the trick as a workaround:

player.setShuffleOrder(shuffleOrder);
Util.createHandlerForCurrentLooper().post(() -> {
    mediaSessionConnector.invalidateMediaSessionQueue();
});

mag0716 · 2022-01-31T05:41:15Z

@marcbaechinger
Thanks for the reply.

If this is true what I'm saying, you should receive the new shuffle order later. When the status update arrives from the playback thread.

As you described, I have confirmed that onQueueChanged is called with the correct song order when I wait for the playback to continue after reorder the songs.

2022-01-31 11:13:50.900 7751-7751/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media3, media4, media6, media7, media5, media0, media1, media8] <- enable shuffle mode
2022-01-31 11:13:55.718 7751-7751/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [2, 4, 3, 6, 7, 5, 0, 1, 8]                             <- move media4 previous to media3
2022-01-31 11:13:55.723 7751-7751/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media3, media4, media6, media7, media5, media0, media1, media8] <- onQueueChanged is called, but not in the correct song order
                                                                                                                                                                    <- continue playing the song
2022-01-31 11:16:36.646 7751-7751/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media4, media3, media6, media7, media5, media0, media1, media8] <- onQueueChanged is called with the correct song order
2022-01-31 11:16:36.741 7751-7751/com.example.android.sampleplayer D/xxx: onQueueChanged : [media2, media4, media3, media6, media7, media5, media0, media1, media8]

This should possibly already do the trick as a workaround:

I try this workaround at mag0716/SamplePlayer@d0d85e5.
More often than not, onQueueChanged is called with the correct song order, but there are cases where onQueueChanged is called with a different song order.

2022-01-31 14:27:48.170 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media1, media2, media5, media8, media3] <- enable shuffle mode
2022-01-31 14:27:52.864 20449-20449/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [7, 4, 6, 0, 1, 2, 5, 3, 8]
2022-01-31 14:27:52.869 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media1, media2, media5, media3, media8] <- success
2022-01-31 14:27:55.359 20449-20449/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [7, 4, 6, 0, 1, 2, 3, 5, 8]
2022-01-31 14:27:55.365 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media1, media2, media3, media5, media8] <- success
2022-01-31 14:27:57.757 20449-20449/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [7, 4, 6, 0, 1, 3, 2, 5, 8]
2022-01-31 14:27:57.763 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media1, media3, media2, media5, media8] <- success
2022-01-31 14:28:00.698 20449-20449/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [7, 4, 6, 0, 3, 1, 2, 5, 8]
2022-01-31 14:28:00.704 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media1, media3, media2, media5, media8]
2022-01-31 14:28:00.757 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media3, media1, media2, media5, media8] <- success, but onQueueChanged is called multiple times
2022-01-31 14:28:00.800 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media3, media1, media2, media5, media8]
2022-01-31 14:28:06.103 20449-20449/com.example.android.sampleplayer D/xxx: set new ShuffleOrder : shuffled = [7, 4, 6, 3, 0, 1, 2, 5, 8]
2022-01-31 14:28:06.109 20449-20449/com.example.android.sampleplayer D/xxx: onQueueChanged : [media7, media4, media6, media0, media3, media1, media2, media5, media8] <- fail

(The following may be inaccurate)

If onQueueChanged is called more than once when reorder the songs, onQueueChanged will be called in the wrong song order when reordering songs next
Often onQueueChanged is called multiple times when the song that next to playing song changes

mag0716 · 2022-02-07T05:30:18Z

I have investigated the cause of this Issue and identified the following as the cause.

The newPlaybackInfo.timeline of ExoPlayerImpl#updatePlaybackInfo called by changing ShuffleOrder reflects the order specified by ShuffleOrder.
EVENT_TIMELINE_CHANGED is added to the queue when timelines are changed, resulting in MediaControllerCompat.Callback#onQueueChanged being called
Uses Timeline#equals to compare Timeline, and also compares Timeline$Window, but does not check the order when shuffle mode is enabled.
Timeline$Window.isDynamic may be changed via ExoPlayerImplInternal#maybeUpdateLoadingPeriod when the media to be loaded is changed by changing the playback state or reordering.
If ExoPlayerImpl#updatePlaybackInfo is called after that, Timeline is determined to have changed and EVENT_TIELINE_CHANGED is queued, so MediaControllerCompat.Callback#onQueueChanged is called.

I have come up with the following two solutions to solve the above problem, and I have tried them and confirmed that the problem in this Issue will be solved.
However, both of them have some disadvantages and I would like your advice.

Also compare order when shuffle mode is enabled in Timeline#equals

diff --git a/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java b/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java
index 8a7c9b5edd..554d176228 100644
--- a/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java
+++ b/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java
+++ b/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java
@@ -1317,6 +1317,15 @@ public abstract class Timeline implements Bundleable {
     if (other.getWindowCount() != getWindowCount() || other.getPeriodCount() != getPeriodCount()) {
       return false;
     }
+    int windowIndex = getFirstWindowIndex(true);
+    int otherWindowIndex = other.getFirstWindowIndex(true);
+    while (windowIndex != C.INDEX_UNSET) {
+      if(windowIndex != otherWindowIndex) {
+        return false;
+      }
+      windowIndex = getNextWindowIndex(windowIndex, Player.REPEAT_MODE_OFF, true);
+      otherWindowIndex = other.getNextWindowIndex(otherWindowIndex, Player.REPEAT_MODE_OFF, true);
+    }
     Timeline.Window window = new Timeline.Window();
     Timeline.Period period = new Timeline.Period();
     Timeline.Window otherWindow = new Timeline.Window();

Timeline#equals is also called from other places, but even if the order of the media when shuffle mode is enabled is not important, it will be processed up to Timeline#getWindowCount()-1 times, which may affect performance.

Force `EVENT_TIMELINE_CHANGED` to be added to the queue if `ShuffleOrder` is changed when shuffle mode is enabled

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
index 83396d6302..4e3d34b6db 100644
         /* ignored */ DISCONTINUITY_REASON_INTERNAL,
         /* ignored */ C.TIME_UNSET,
         /* ignored */ C.INDEX_UNSET);
+    if(shuffleModeEnabled) {
+      eventListeners.queueEvent(
+          Player.EVENT_TIMELINE_CHANGED,
+          listener -> listener.onTimelineChanged(
+              newPlaybackInfo.timeline,
+              TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED
+          )
+      );
+    }
   }
 
   public void setPauseAtEndOfMediaItems(boolean pauseAtEndOfMediaItems) {

The impact is limited, but the code for ExoPlayerImpl#setShuffleOrder becomes more complex.

I would like to create a PR if any of the above proposed solutions seem to work.
Could you please check it? Thanks.

mag0716 · 2022-05-16T04:26:04Z

@marcbaechinger

Just following up on this issue, are there any updates?
We are currently using folked repository(modify Timeline#equals) as a workaround, but we would rather use the original ExoPlayer.

Previously two timelines that differed only in shuffle order were considered equal, which resulted in no call to Player.Listener.onTimelineChanged when calling ExoPlayer.setShuffleOrder. This in turn resulted in no call to MediaControllerCompat.Callback.onQueueChanged. Also make a small fix inside ExoPlayerImpl.setShuffleOrder, to ensure that the new shuffle order is used when constructing the masked timeline. Issue: google/ExoPlayer#9889 #minor-release PiperOrigin-RevId: 457703727

Previously two timelines that differed only in shuffle order were considered equal, which resulted in no call to Player.Listener.onTimelineChanged when calling ExoPlayer.setShuffleOrder. This in turn resulted in no call to MediaControllerCompat.Callback.onQueueChanged. Also make a small fix inside ExoPlayerImpl.setShuffleOrder, to ensure that the new shuffle order is used when constructing the masked timeline. Issue: #9889 #minor-release PiperOrigin-RevId: 457703727

tonihei · 2022-07-15T08:25:20Z

Fixed by the commit above.

Previously two timelines that differed only in shuffle order were considered equal, which resulted in no call to Player.Listener.onTimelineChanged when calling ExoPlayer.setShuffleOrder. This in turn resulted in no call to MediaControllerCompat.Callback.onQueueChanged. Also make a small fix inside ExoPlayerImpl.setShuffleOrder, to ensure that the new shuffle order is used when constructing the masked timeline. Issue: #9889 #minor-release PiperOrigin-RevId: 457703727 (cherry picked from commit 5c7ec13)

Previously two timelines that differed only in shuffle order were considered equal, which resulted in no call to Player.Listener.onTimelineChanged when calling ExoPlayer.setShuffleOrder. This in turn resulted in no call to MediaControllerCompat.Callback.onQueueChanged. Also make a small fix inside ExoPlayerImpl.setShuffleOrder, to ensure that the new shuffle order is used when constructing the masked timeline. Issue: google/ExoPlayer#9889 #minor-release PiperOrigin-RevId: 457703727 (cherry picked from commit 6f9ce40)

mag0716 added needs triage question labels Jan 24, 2022

marcbaechinger self-assigned this Jan 24, 2022

marcbaechinger added need more info and removed needs triage labels Jan 24, 2022

google-oss-bot added needs triage and removed need more info labels Jan 26, 2022

marcbaechinger added bug and removed question needs triage labels Jan 30, 2022

tonihei closed this as completed Jul 15, 2022

Tolriq mentioned this issue Jul 27, 2022

evaluateMediaItemTransitionReason - IllegalStateException #10244

Closed

google locked and limited conversation to collaborators Sep 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

How to support reordering during the shuffle #9889

How to support reordering during the shuffle #9889

mag0716 commented Jan 24, 2022

marcbaechinger commented Jan 24, 2022

mag0716 commented Jan 26, 2022

mag0716 commented Jan 26, 2022

marcbaechinger commented Jan 30, 2022 •

edited

Loading

mag0716 commented Jan 31, 2022

mag0716 commented Feb 7, 2022 •

edited

Loading

mag0716 commented May 16, 2022

tonihei commented Jul 15, 2022

How to support reordering during the shuffle #9889

How to support reordering during the shuffle #9889

Comments

mag0716 commented Jan 24, 2022

Things I want to do

Current approach

Result

Environment

Sample Project

Question

marcbaechinger commented Jan 24, 2022

mag0716 commented Jan 26, 2022

mag0716 commented Jan 26, 2022

Results

marcbaechinger commented Jan 30, 2022 • edited Loading

mag0716 commented Jan 31, 2022

mag0716 commented Feb 7, 2022 • edited Loading

Also compare order when shuffle mode is enabled in Timeline#equals

Force EVENT_TIMELINE_CHANGED to be added to the queue if ShuffleOrder is changed when shuffle mode is enabled

mag0716 commented May 16, 2022

tonihei commented Jul 15, 2022

marcbaechinger commented Jan 30, 2022 •

edited

Loading

mag0716 commented Feb 7, 2022 •

edited

Loading

Force `EVENT_TIMELINE_CHANGED` to be added to the queue if `ShuffleOrder` is changed when shuffle mode is enabled