Provide more detailed discontinuity reasons

[greyski]

[REQUIRED] Use case description

Related to another issue #4898 specifically #4898 (comment)

I've consolidated the source cited in the comment:

private int previousWindowIndex;

void onPositionDiscontinuity(int reason) { // AnalyticsListener.onPositionDiscontinuity
  if (reason == DISCONTINUITY_REASON_PERIOD_TRANSITION) {
    if (previousWindowIndex == player.getCurrentWindowIndex() {
      // Repeating the same playlist item automatically.
    } else {
      // Moved to next playlist item automatically.
    }
  } else if (reason == DISCONTINUITY_REASON_SEEK) {
    if (previousWindowIndex == player.getCurrentWindowIndex() {
      // Seek within current playlist item.
    } else {
      // Seek to another playlist item (e.g. skipping to next or previous)
    }
  } else if (reason == DISCONTINUITY_REASON_AD_INSERTION) {
    // Transitioned from content to ad or from ad to content within current playlist item.
  } else {
    // Position jumped discontinuously within current playlist item. For example because
    // seek got adjusted or for other internal reason.
  }
  previousWindowIndex = player.getCurrentWindowIndex();
}

There are many cases where we do not have immediate access to the Player when we're implementing an AnalyticsListener for event tracking and it feels odd to have to hold a reference to the Player within it's own listener in order to track, what seems like, events that should be provided out-of-the-box (like when a video loops or when seeking to another item in a playlist).

Proposed solution

I could be incorrect, but I believe since these events are being fired from ExoPlayerImpl, we should be able to return events specific to when previousWindowIndex == getCurrentWindowIndex() as the method getCurrentWindowIndex() implementation can be found inside of the ExoPlayerImpl class. Therefore we can include additional DiscontinuityReasons so the check within onPositionDiscontinuity can be setup as so:

void onPositionDiscontinuity(int reason) { // AnalyticsListener.onPositionDiscontinuity
  if (reason == DISCONTINUITY_REASON_PERIOD_TRANSITION) {
      // Repeating the same playlist item automatically.
  } else if (reason == DISCONTINUITY_REASON_PERIOD_TRANSITION_NEW_WINDOW) {
    // Moved to next playlist item automatically.
  } else if (reason == DISCONTINUITY_REASON_SEEK) {
    // Seek within current playlist item.
  } else if (reason == DISCONTINUITY_REASON_SEEK_NEW_WINDOW) {
    // Seek to another playlist item (e.g. skipping to next or previous)
  } else if (reason == DISCONTINUITY_REASON_AD_INSERTION) {
    // Transitioned from content to ad or from ad to content within current playlist item.
  } else {
    // Position jumped discontinuously within current playlist item. For example because
    // seek got adjusted or for other internal reason.
  }
}

This removes the need to house a reference to the Player within its own AnalyticsListener implementation.

Alternatives considered

n/a

[tonihei]

Actually, AnalyticsListener also does that for you and the EventTime passed to every event contains the current window index without requiring a Player reference.

Besides that, it's indeed worthwhile thinking about more detailed discontinuity reasons. As you pointed out, it's easy to distinguish between standard transition to a new window and repeating the same window. Similarly, listeners often need to distinguish between seek within item and seeking to another item. And finally, it's slightly awkward to remember that onTimelineChanged may include an implicit discontinuity. It's easy to forget and you can't see if it actually happened as most timeline updates will probably not include a position discontinuity. That means it would be good to add a new reason for Timeline change triggered discontinuities and call this method automatically. I'll rename this enhancement to track improving the reasons.

[greyski]

Oh cool, I did not know EventTime housed this information.

I agree with including more explicit DiscontinuityReasons.

Thanks for the continued fast response to these requests/questions!

[tonihei]

This has been partly addressed in the latest version (2.12.0) by adding a onMediaItemTransition callback that gets called for every change to the current MediaItem, including start-up and auto-repeat. There is a separate reason for REPEAT to easily distinguish this case.

Remaining work:

• Separate reasons for seek within a window or to another window.
  • Won't be implemented because we will address Need a callback for "on just before position discontinuity frame"? #6576 to add the position before the discontinuity to the callback. This way implementations can easily distinguish between these two cases without additional local variables to save state.
• Calling onPositionDiscontinuity for cases where a discontinuity implicitly happened as part of a onTimelineChanged callback.
  • We will address that soon too to more clearly differentiate the two listeners and to make sure implementations only need to override one method.

[marcbaechinger]

This commit that landed in dev-v2 deprecates the old onPositionDiscontinuity callback in favor of a new version that has two additional parameters for the old and new PositionInfo.

I think this solves some of the problems described in this issue.

• the index/uid of the window of the position before and after the discontinuity can be checked in the PositionInfo objects passed as parameters. This makes it easy to know whether a DISCONTINUITY_REASON_AUTO_TRANSITION is a transition to the next window or repeating the same window.
• there is a new reason Player.DISCONTINUITY_REASON_REMOVE which indicates that a discontinuity has been caused by a playlist update (this makes listening for these cases in onTimlineChanged obsolete)