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

Potential for Multiple Non-Internal Discontinuity Events #1483

Open
1 task
skidder opened this issue Jun 22, 2024 · 1 comment
Open
1 task

Potential for Multiple Non-Internal Discontinuity Events #1483

skidder opened this issue Jun 22, 2024 · 1 comment
Assignees
Labels

Comments

@skidder
Copy link

skidder commented Jun 22, 2024

Version

Media3 1.1.1 / ExoPlayer 2.19.1

More version details

No response

Devices that reproduce the issue

Observed on Android OS 11-14, could include more.

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

  1. Implement a playback scenario with multiple periods or ads, ensuring the playback queue has distinct playing and reading periods.
  2. Induce a renderer error during playback, specifically when the playing period differs from the reading period.
  3. Observe the handling of discontinuity events following the renderer error, particularly the reporting of non-internal discontinuity reasons (Player.DISCONTINUITY_REASON_AUTO_TRANSITION) in quick succession or across different parts of the playback queue.

It's also manually reproducible in unit tests by instantiating PlaybackInfoUpdate and invoking setPositionDiscontinuity multiple times with non-internal events, i.e.

    // rapidly set position discontinuity
    playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK);
    playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL);
    playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK);

Here's a draft PR showing the issue google/ExoPlayer#11392

Expected result

No assert or unit test failures.

Actual result

In production we've seen app crashes with the following stack-trace:

java.lang.IllegalArgumentException: null
    at com.google.android.exoplayer2.util.Assertions.checkArgument
    at com.google.android.exoplayer2.ExoPlayerImplInternal$PlaybackInfoUpdate.setPositionDiscontinuity(SourceFile:121)
    at com.google.android.exoplayer2.ExoPlayerImplInternal.handlePositionDiscontinuity(SourceFile:2489)
    at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(SourceFile:626)
    at android.os.Handler.dispatchMessage(Handler.java:102)
    at android.os.Looper.loopOnce(Looper.java:210)
    at android.os.Looper.loop(Looper.java:299)
    at android.os.HandlerThread.run(HandlerThread.java:67)

We began seeing this exception after upgrading from Exoplayer 2.13.3 to 2.19.1. This stack-trace follows a path introduced in commit 79b688e. It occurs across a broad range of Android OS (mostly 11-14) and Android device class.

Media

Not applicable

Bug Report

@tonihei
Copy link
Collaborator

tonihei commented Jun 25, 2024

Thanks for reporting! Reporting multiple non-internal discontinuities is not expected to happen within the same iteration generally, but the logic added in 79b688e is special in that it can run after nearly every other piece of code in this class and needs to handle the potential for other pending events.

copybara-service bot pushed a commit that referenced this issue Jun 26, 2024
When handling a playback error that originates from a future item in
the playlist, we added support for jumping to that item first,
ensuring the errors 'happen' for the right 'current item'.
See 79b688e.

However, when we add this new position discontinuity to the
playback state, there may already be other position discontinuities
pending from other parts of the code that executed before the
error. As we can't control that in this case (because it's part
of a generic try/catch block), we need to send any pending
updates first before handling the new change.

Issue: #1483
#cherrypick
PiperOrigin-RevId: 646968309
tianyif pushed a commit that referenced this issue Jul 2, 2024
When handling a playback error that originates from a future item in
the playlist, we added support for jumping to that item first,
ensuring the errors 'happen' for the right 'current item'.
See 79b688e.

However, when we add this new position discontinuity to the
playback state, there may already be other position discontinuities
pending from other parts of the code that executed before the
error. As we can't control that in this case (because it's part
of a generic try/catch block), we need to send any pending
updates first before handling the new change.

Issue: #1483
PiperOrigin-RevId: 646968309
(cherry picked from commit 7276451)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants