Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getCurrentTag fails with IndexOutOfBoundException #4822

Closed
thibseisel opened this issue Sep 16, 2018 · 1 comment
Closed

getCurrentTag fails with IndexOutOfBoundException #4822

thibseisel opened this issue Sep 16, 2018 · 1 comment
Assignees
Labels

Comments

@thibseisel
Copy link

Issue description

In the Player.Listener.onTimelineChanged() callback, I query for the current window tag by calling player.getCurrentTag(). In my use case, it fails with an IndexOutOfBoundException. When debuging into ExoPlayerImpl.getCurrentTag() ...

@Override
  public @Nullable Object getCurrentTag() {
    int windowIndex = getCurrentWindowIndex();
    return windowIndex > playbackInfo.timeline.getWindowCount()
        ? null
        : playbackInfo.timeline.getWindow(windowIndex, window, /* setTag= */ true).tag;
  }

... I noticed that this could happen if getCurrentWindowIndex == playbackInfo.timeline.getWindowCount(), leading playbackInfo.timeline.getWindow(...) to fail.
In my opinion, this should be avoided by using the "greater or equal" sign instead of the "strictly greater" in the condition, returning null instead of failing with an exception.

I don't understand why this is happening in the first place, because onTimelineChanged was called as a result of preparing a new ConcatenatingMediaSource, the current window index should not be out of range.

Here is the stacktrace:

09-16 19:09:27.307 W/System.err: Caused by: java.lang.IndexOutOfBoundsException
09-16 19:09:27.308 W/System.err:     at com.google.android.exoplayer2.Timeline$1.getWindow(Timeline.java:506)
09-16 19:09:27.309 W/System.err:     at com.google.android.exoplayer2.Timeline.getWindow(Timeline.java:633)
09-16 19:09:27.310 W/System.err:     at com.google.android.exoplayer2.ExoPlayerImpl.getCurrentTag(ExoPlayerImpl.java:334)
09-16 19:09:27.311 W/System.err:     at com.google.android.exoplayer2.SimpleExoPlayer.getCurrentTag(SimpleExoPlayer.java:767)
09-16 19:09:27.312 W/System.err:     at fr.nihilus.music.media.playback.MyQueueNavigator.onUpdateMediaSessionMetadata(MyQueueNavigator.kt:147)
09-16 19:09:27.313 W/System.err:     at fr.nihilus.music.media.playback.MyQueueNavigator.onTimelineChanged(MyQueueNavigator.kt:134)
        at com.google.android.exoplayer2.ext.mediasession.MediaSessionConnector$ExoPlayerEventListener.onTimelineChanged(MediaSessionConnector.java:682)
09-16 19:09:27.314 W/System.err:     at com.google.android.exoplayer2.ExoPlayerImpl$PlaybackInfoUpdate.notifyListeners(ExoPlayerImpl.java:746)
09-16 19:09:27.315 W/System.err:     at com.google.android.exoplayer2.ExoPlayerImpl.updatePlaybackInfo(ExoPlayerImpl.java:681)
        at com.google.android.exoplayer2.ExoPlayerImpl.prepare(ExoPlayerImpl.java:187)
09-16 19:09:27.316 W/System.err:     at com.google.android.exoplayer2.SimpleExoPlayer.prepare(SimpleExoPlayer.java:688)
        at com.google.android.exoplayer2.SimpleExoPlayer.prepare(SimpleExoPlayer.java:675)

Reproduction steps

In an audio app that uses the ExoPlayer MediaSession Extension with MediaSessionConnector in a MediaBrowserServiceCompat (this may not be a requirement to use the extension, though) ...

  1. Prepare a ConcatenatingMediaSource consisting of multiple ExtractorMediaSources, each (preferably) pointing to the Uri of a music file on the device's external storage.
  2. Wait for it to be fully prepared.
  3. Register a Player.Listener instance that calls player.getCurrentTag() in the onTimelineChanged() callback.
  4. prepare another ConcatenatingMediaSource (that may consist of the same MediaSources as above)
  5. An IndexOutOfBoundsException should be thrown when calling getCurrentTag() in onTimelineChanged().

Version of ExoPlayer being used

2.8.4

Device(s) and version(s) of Android being used

LG Nexus 5X, API 27 (Oreo MR1)

@tonihei
Copy link
Collaborator

tonihei commented Sep 17, 2018

In my opinion, this should be avoided by using the "greater or equal" sign instead of the "strictly greater" in the condition, returning null instead of failing with an exception.

Agree. That should definitely be a "greater or equal" sign. We'll fix that.

I don't understand why this is happening in the first place, because onTimelineChanged was called as a result of preparing a new ConcatenatingMediaSource, the current window index should not be out of range.

If you prepare the player with a new media source (step 4 in your reproduction steps), the timeline is unknown until the new media source tells the player its media structure. You'll notice that the timeline reported after calling prepare again is empty (and the timeline change reason is TIMELINE_CHANGE_REASON_RESET).

If you want to keep the current timeline while re-preparing, please pass false to the resetState parameter of player.prepare. Note that this is only advisable if you use the same media source and the timeline is indeed still correct.

ojw28 pushed a commit that referenced this issue Sep 17, 2018
To check the validity of a window index it needs to be compared with a greater
or equal sign to the window count.

Issue:#4822

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=213234403
@ojw28 ojw28 closed this as completed Sep 17, 2018
@google google locked and limited conversation to collaborators Jan 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants