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

How to correctly remove PlayerMessage? #7278

Closed
Kamil-H opened this issue Apr 22, 2020 · 5 comments
Closed

How to correctly remove PlayerMessage? #7278

Kamil-H opened this issue Apr 22, 2020 · 5 comments
Assignees
Labels

Comments

@Kamil-H
Copy link

Kamil-H commented Apr 22, 2020

Searched documentation and description of the problem

I'm developing an app where I let user listen to some audio content. My content is built with ConcatenatingMediaSource and provided to the user as a playlist. There are advertisements baked into my content, HLS files. In the response from the API I have information when exactly the ad begins, when user reached middle of the ad and the end of the ad. I would like to be able to send send a data to the API in those points in timeline. I have checked all of the possible options in library like AdsMediaSource, ClippingMediaSource and nothing matches my requirement as well as PlayerMessage. It lets me set a "points" in my media files and be notified when user reached one.
I'm using following methods to set a time when message should be sent:

.setPayload(reportingData)
.setPosition(positionInPlaylist, (reportingData.position * SECONDS_TO_MILLISECONDS_MULTIPLIER).toLong())
.setDeleteAfterDelivery(false)
.setType(ADVERTISEMENT_MESSAGE_TYPE)
.send()

From what I observed, PlayerMessage is assigned to windowIndex (media on my playlist) and it's great, because when I for example add message to 4th windowIndex then remove 3rd item and I'm starting playing next one (the on that have message assigned), the message is delivered.
User can remove items in playlist and add them again. So ideally it would be to react on Timeline change and remove all of the messages and add it again, to make 100% sure that those are added in correct points of timeline.
My problem is that when I'm trying cancel a current messages:

playerMessages.forEach { it.cancel() }
playerMessages.clear()

and then I'm adding them again in current positions, I'm getting following error:

E/ExoPlayerImplInternal: Internal runtime error.
    java.lang.IndexOutOfBoundsException: Index: 14, Size: 1
        at java.util.ArrayList.get(ArrayList.java:437)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.maybeTriggerPendingMessages(ExoPlayerImplInternal.java:1081)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.updatePlaybackPositions(ExoPlayerImplInternal.java:553)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:583)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:329)
        at android.os.Handler.dispatchMessage(Handler.java:103)
        at android.os.Looper.loop(Looper.java:214)
        at android.os.HandlerThread.run(HandlerThread.java:67)

The error is not exactly related to what I'm doing, but is always happening when I'm deleting a MediaSource from the playlist that had PlayerMessage and I'm adding it again and trying to add its PlayerMessages

From what I can see in the crash report it says "IndexOutOfBoundsException: Index: 14, Size: 1", so it seems like the messages were removed from the list, but it somehow tries to iterate through them with higher index than the number of the items currently on the list.

Question

My question is: is it correct use case of the PlayerMessage? Maybe there is a better tool for this use case? Ideally it would be if I could add the "points" to MediaSource directly and be notified when user reached one, but I didn't find a way to do that.
It I'm not abusing PlayerMessage, then why I'm getting an error when trying to add them more?

@Kamil-H
Copy link
Author

Kamil-H commented Apr 22, 2020

I investigated a problem a little bit more and I can say that I have a different problem. I'm getting a crash every time I'm doing following flow:

  1. Add a PlayerMessage on the windowIndex
  2. Play a media source at this windowIndex handle PlayerMessage
  3. Switch to next item
  4. Remove media source at windowIndex
  5. Add again media source removed before
  6. Try to add PlayerMessage on the media source added in last step

Here is the crash report I'm getting (I had 4 PlayerMessages added to this media source I removed and tried to add again)

E/ExoPlayerImplInternal: Internal runtime error.
    java.lang.IndexOutOfBoundsException: Index: 4, Size: 1
        at java.util.ArrayList.get(ArrayList.java:437)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.maybeTriggerPendingMessages(ExoPlayerImplInternal.java:1081)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.updatePlaybackPositions(ExoPlayerImplInternal.java:553)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:583)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:329)
        at android.os.Handler.dispatchMessage(Handler.java:103)
        at android.os.Looper.loop(Looper.java:214)
        at android.os.HandlerThread.run(HandlerThread.java:67)

It's even more interesting, because ExoPlayer crashes and playback is getting stopped, but those PlayerMessages are getting added to the Media source and after preparing again it's playing and PlayerMessages are delivered correctly.

@tonihei
Copy link
Collaborator

tonihei commented Apr 23, 2020

My question is: is it correct use case of the PlayerMessage?

Yes, definitely! That's what it was designed for.

I'm using following methods to set a time when message should be sent:

Not sure if it is intentional, but this setup delivers the messages on the playback thread. If you rather like them to be delivered on another thread (e.g. the main thread), please use PlayerMessage.setHandler.

I'm getting a crash every time I'm doing following flow:

Can you provide code snippets for each step and additional notes if other thread etc are involved? Some steps are a bit ambiguous based on this description. Thanks!

@Kamil-H
Copy link
Author

Kamil-H commented Apr 24, 2020

@tonihei thank you for your answer. I decided to create a simple project that demonstrates a problem:
https://github.com/Kamil-H/ExoPlayer-Crash
Basically it's a simplified version of setup of the app I'm working on currently.
Steps to reproduce a problem:

  1. Run the app
  2. Switch to "Voyage I - Waterfall"
  3. Switch to next
  4. Swipe to delete "Voyage I - Waterfall"
  5. Undo this removal with a button on Snackbar that appear
    I also created a simple video demonstrating this behavior:
    https://drive.google.com/file/d/1Dz0dhf2J6rRT2c0vvtkPMa4TAUaOA78R/view?usp=sharing
    as you can see after removing item and undo this ExoPlayer crashes with:
2020-04-24 18:32:46.258 30951-30990/com.nomtek.exoplayercrash E/ExoPlayerImplInternal: Internal runtime error
      java.lang.IndexOutOfBoundsException: Index: 4, Size: 2
        at java.util.ArrayList.get(ArrayList.java:437)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.maybeTriggerPendingMessages(ExoPlayerImplInternal.java:1081)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.updatePlaybackPositions(ExoPlayerImplInternal.java:553)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:583)
        at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:329)
        at android.os.Handler.dispatchMessage(Handler.java:103)
        at android.os.Looper.loop(Looper.java:214)
        at android.os.HandlerThread.run(HandlerThread.java:67)

report.
As you can see in AdvertisementSetter I added .setHandler(Handler(Looper.getMainLooper())) line, but it didn't help me.

To make it more clear - items on the list with "Ad included" are a media items that have PlayerMessages added.

I did more investigation and when I have several items with "ads" (PlayerMessages in fact) on the playlist, the whole problem occurs only for the last item with "ads". So let's say I have 5 items on the playlist with PlayerMessages. I can remove and insert again first four items without crash, but it happens when I try it with the 5th one.

Let me know if this description and project is clear.

Thank you in advance!

@tonihei
Copy link
Collaborator

tonihei commented Apr 27, 2020

Thanks a lot for the reproduction steps and the project. I can reproduce the problem and will provide a fix.

@Kamil-H
Copy link
Author

Kamil-H commented Apr 27, 2020

Thank you.

ojw28 pushed a commit that referenced this issue May 1, 2020
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
@ojw28 ojw28 closed this as completed May 14, 2020
ojw28 pushed a commit that referenced this issue May 28, 2020
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
andrewlewis pushed a commit that referenced this issue Jun 1, 2020
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
@google google locked and limited conversation to collaborators Jul 14, 2020
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

4 participants