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.

Sign up for GitHub

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

Jump to bottom

onPlayerStateChanged is not consistent for multiple listeners #4276

Closed
fcapinho opened this issue May 21, 2018 · 1 comment
Closed

onPlayerStateChanged is not consistent for multiple listeners #4276

fcapinho opened this issue May 21, 2018 · 1 comment
Assignees
@tonihei
Labels
bug

Comments

@fcapinho
Copy link

Issue description

State updates from Player.EventListener.onPlayerStateChanged() may be inconsistent between multiple listeners added with Player.addListener(). This happens when one of the listeners callbacks changes the Player state.

For example, if a listener call Player.stop() when receiving a Player.STATE_ENDED state update, all succeeding listeners will receive a Player.STATE_IDLE state update.
The Expected behaviour is that all listeners receive the same state update values.

Reproduction steps

  1. Apply this patch to the main demo app in order to add two new Player.EventListener:
  2. Play any video from the main demo app. The following logcat snippet is from full play of "YouTube DASH -> Google Play (WebM, VP9)":
05-21 01:16:58.016 16603-16603/com.google.android.exoplayer2.demo D/PlayerEndEventListener: playbackState: IDLE / playyWhenReady: true
05-21 01:16:58.017 16603-16603/com.google.android.exoplayer2.demo D/PlayerLogEventListener: playbackState: IDLE / playyWhenReady: true
05-21 01:16:58.208 16603-16603/com.google.android.exoplayer2.demo D/PlayerEndEventListener: playbackState: BUFFERING / playyWhenReady: true
05-21 01:16:58.208 16603-16603/com.google.android.exoplayer2.demo D/PlayerLogEventListener: playbackState: BUFFERING / playyWhenReady: true
05-21 01:17:00.643 16603-16603/com.google.android.exoplayer2.demo D/PlayerEndEventListener: playbackState: READY / playyWhenReady: true
05-21 01:17:00.643 16603-16603/com.google.android.exoplayer2.demo D/PlayerLogEventListener: playbackState: READY / playyWhenReady: true
05-21 01:18:01.904 16603-16603/com.google.android.exoplayer2.demo D/PlayerEndEventListener: playbackState: ENDED / playyWhenReady: true
05-21 01:18:01.905 16603-16603/com.google.android.exoplayer2.demo D/PlayerEndEventListener: playbackState: IDLE / playyWhenReady: true
05-21 01:18:01.905 16603-16603/com.google.android.exoplayer2.demo D/PlayerLogEventListener: playbackState: IDLE / playyWhenReady: true
05-21 01:18:01.921 16603-16603/com.google.android.exoplayer2.demo D/PlayerLogEventListener: playbackState: IDLE / playyWhenReady: true

The issue is on the latest 4 state updates. Note that PlayerLogEventListener never receives the Player.STATE_ENDED state upate:

PlayerEndEventListener: playbackState: ENDED
PlayerEndEventListener: playbackState: IDLE
PlayerLogEventListener: playbackState: IDLE
PlayerLogEventListener: playbackState: IDLE

Link to test content

Can be reproduced with all medias from main demo app.

Version of ExoPlayer being used

r2.8.0

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

Reproduced on Galaxy S7 and Motorola G5, both running Android 7.0 (API 24), and on Emulator running Android 8.1.0 (API 27).

A full bug report captured from the device

bugreport-sdk_gphone_x86-OSM1.180201.007-2018-05-21-01-54-13.zip
@tonihei
Copy link
Collaborator

tonihei commented May 21, 2018

We are aware of this issue and already have plans to fix this. Marking as bug until it's fixed.

@tonihei tonihei added the bug label May 21, 2018
@tonihei tonihei self-assigned this May 21, 2018
ojw28 pushed a commit that referenced this issue Jun 5, 2018
@tonihei @ojw28
Serialize recursive listener notifications.
0912fd1 
When the player state is changed from an event listener callback, we may
get recursive listener notifications. These recursions can produce a wrong
order, skip or duplicate updates, and send different notifications to
different listeners.

This change serializes listener notifications by clustering all update data
in a helper data class and adding the updates to a queue which can be handled
in a loop on the outer layer of the recursion.

As playWhenReady updates also reference the current playbackInfo, we need to
redirect the listener notifcations for setPlayWhenReady to the same queue.

Issue:#4276

-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=198031431
@ojw28 ojw28 closed this as completed Jun 5, 2018
ojw28 pushed a commit that referenced this issue Jun 5, 2018
@tonihei @ojw28
Serialize recursive listener notifications.
9de9678 
When the player state is changed from an event listener callback, we may
get recursive listener notifications. These recursions can produce a wrong
order, skip or duplicate updates, and send different notifications to
different listeners.

This change serializes listener notifications by clustering all update data
in a helper data class and adding the updates to a queue which can be handled
in a loop on the outer layer of the recursion.

As playWhenReady updates also reference the current playbackInfo, we need to
redirect the listener notifcations for setPlayWhenReady to the same queue.

Issue:#4276

-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=198031431
@google google locked and limited conversation to collaborators Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tonihei tonihei

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ojw28 @fcapinho @tonihei