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

Seek to default live does not restore idealTargetLiveOffset #11050

Closed
1 task
stevemayhew opened this issue Mar 9, 2023 · 6 comments
Closed
1 task

Seek to default live does not restore idealTargetLiveOffset #11050

stevemayhew opened this issue Mar 9, 2023 · 6 comments
Assignees
Labels

Comments

@stevemayhew
Copy link
Contributor

ExoPlayer Version

2.18.4

Devices that reproduce the issue

  • SEI500 SEI Robotics (AmLogic SoC STB) running Android 29
  • Probably everything else with AAC audio

Devices that do not reproduce the issue

None I'm aware of.

Reproducible in the demo app?

Yes

Reproduction steps

  1. Play live stream with non-default LiveConfiguration
  2. Seek back (jump back 5 button) a few times, notice setTargetLiveOffsetOverrideUs() is called to move the target (not sure why this is a feature, IMO live adjustment should shut off here until seek back to live edge)
  3. call seekTo(C.TIME_UNSET) (from the UI play next button with no next item will do this.

Expected result

Live adjustment resumes (continues) using the original target set on playback startup (mediaConfigurationTargetLiveOffsetUs)

Actual result

Playback seeks to the live edge, but Live adjustment desperately tries to move back to the last non default seek.

Logging shows clearly what is happening:

Step 1 - normal startup and live adjustment

03-09 14:47:30.744 20204 20204 I ExoPlayerImpl: Init c9a8d43 [ExoPlayerLib/2.18.3] [SEI500RCN, Pyxis-RCN, SEI Robotics, 29]
03-09 14:47:31.665 20204 21891 D DefaultLivePlaybackSpeedControl: setLiveConfiguration(LiveConfiguration - min/max speed: 0.9/1.1 targetOffset: 33284) - updated
03-09 14:47:33.414 20204 21891 D DefaultLivePlaybackSpeedControl: adjusting offset, speed: 1.1 liveOffsetUs: 38107723 currentTargetLiveOffsetUs: 33284000 idealTargetLiveOffsetUs: 33284000 smoothedMinPossibleLiveOffsetUs: 32112240

Step 2 - three back button presses jump back 15 seconds total, override is called (setTargetLiveOffsetOverrideUs())

03-09 14:48:02.377 20204 21891 D ExoPlayerImplInternal: seekToInternal() - timeline empty: false  seekPosition.windowPositionUs: 3575642000
03-09 14:48:02.378 20204 21891 D ExoPlayerImplInternal: resolvedSeekPosition periodUid: Pair{java.lang.Object@9be3c3e java.lang.Object@c575286} resolvedContentPositionUs: 3599666000
03-09 14:48:02.379 20204 21891 D ExoPlayerImplInternal: before seekToPeriodPosition() - newPeriodPositionUs: 3599666000 seekTargetWindow.defaultPositionUs: 3573570000 playbackInfo.positionUs 3604666116 requestedContentPositionUs: 3599666000
03-09 14:48:02.430 20204 21891 D DefaultLivePlaybackSpeedControl: setTargetLiveOffsetOverrideUs(41042000)
03-09 14:48:03.035 20204 21891 D ExoPlayerImplInternal: seekToInternal() - timeline empty: false  seekPosition.windowPositionUs: 3564636000
03-09 14:48:03.035 20204 21891 D ExoPlayerImplInternal: resolvedSeekPosition periodUid: Pair{java.lang.Object@9be3c3e java.lang.Object@c575286} resolvedContentPositionUs: 3594666000
03-09 14:48:03.036 20204 21891 D ExoPlayerImplInternal: before seekToPeriodPosition() - newPeriodPositionUs: 3594666000 seekTargetWindow.defaultPositionUs: 3573570000 playbackInfo.positionUs 3599666000 requestedContentPositionUs: 3594666000
03-09 14:48:03.085 20204 21891 D DefaultLivePlaybackSpeedControl: setTargetLiveOffsetOverrideUs(46697000)
03-09 14:48:03.832 20204 21891 D ExoPlayerImplInternal: seekToInternal() - timeline empty: false  seekPosition.windowPositionUs: 3559636000
03-09 14:48:03.832 20204 21891 D ExoPlayerImplInternal: resolvedSeekPosition periodUid: Pair{java.lang.Object@9be3c3e java.lang.Object@c575286} resolvedContentPositionUs: 3589666000
03-09 14:48:03.832 20204 21891 D ExoPlayerImplInternal: before seekToPeriodPosition() - newPeriodPositionUs: 3589666000 seekTargetWindow.defaultPositionUs: 3573570000 playbackInfo.positionUs 3594666000 requestedContentPositionUs: 3589666000
03-09 14:48:03.865 20204 21891 D DefaultLivePlaybackSpeedControl: setTargetLiveOffsetOverrideUs(52477000)
03-09 14:48:05.323 20204 21891 D DefaultLivePlaybackSpeedControl: adjusting offset, speed: 1.1 liveOffsetUs: 53930908 currentTargetLiveOffsetUs: 52477000 idealTargetLiveOffsetUs: 52477000 smoothedMinPossibleLiveOffsetUs: 48192019
03-09 14:48:06.328 20204 21891 D DefaultLivePlaybackSpeedControl: adjusting offset, speed: 1.1 liveOffsetUs: 53941958 currentTargetLiveOffsetUs: 52477000 idealTargetLiveOffsetUs: 52477000 smoothedMinPossibleLiveOffsetUs: 47873236

Step 3 - seek default is executed (prepare() will do this as well), notice setTargetLiveOffsetOverrideUs(C.TIME_UNSET) is not called so adjustment reverses trying (incorrectly) to slow down to the old seek position

03-09 14:48:18.337 20204 21891 D ExoPlayerImplInternal: seekToInternal() - timeline empty: false  seekPosition.windowPositionUs: -9223372036854775807
03-09 14:48:18.337 20204 21891 D ExoPlayerImplInternal: resolvedSeekPosition periodUid: Pair{java.lang.Object@9be3c3e java.lang.Object@c575286} resolvedContentPositionUs: 3621618011
03-09 14:48:18.338 20204 21891 D ExoPlayerImplInternal: before seekToPeriodPosition() - newPeriodPositionUs: 3621618011 seekTargetWindow.defaultPositionUs: 3579576000 playbackInfo.positionUs 3603559763 requestedContentPositionUs: -9223372036854775807
03-09 14:48:19.126 20204 21891 D DefaultLivePlaybackSpeedControl: adjusting offset, speed: 0.9 liveOffsetUs: 35776574 currentTargetLiveOffsetUs: 52745177 idealTargetLiveOffsetUs: 52477000 smoothedMinPossibleLiveOffsetUs: 28691196

Media

Will send a live test stream to the developer email.

Bug Report

stevemayhew added a commit to TiVo/ExoPlayer that referenced this issue Mar 10, 2023
Any calls that issue a `seekTo()` where the position is `TIME_UNSET` will
now restore the live offset target to the MediaItem set default.

The code handles an intra-period seek in the same timeline by simply updating or removing
the `setTargetLiveOffsetOverrideUs()`.  Other cases call the original code.

This fixes issue google#11050
@stevemayhew
Copy link
Contributor Author

@tonihei I'm guessing this is all yours. The feature works mostly great for us but this fix is a must.

Here's the branch that has the debug logging: https://github.com/TiVo/ExoPlayer/tree/x-debug-for-issue-11050

I'll open a pull request with this possible fix (it works for all our uses cases): https://github.com/TiVo/ExoPlayer/tree/p-fix-for-issue-11050

Thanks!

@stevemayhew
Copy link
Contributor Author

Pull request #11051 fixes the issue for us. All the test cases pass w or w/o the fix so not covered. Let me know if the fix is ok I can have someone update test cases and add them to the pull request.

@stevemayhew
Copy link
Contributor Author

@marcbaechinger Thanks for taking this on, the substantial population of our installed base is watching live, HLS, with ExoPlayer so this is an important new feature for us! Thanks for the excellent first implementation.

There are a couple of uses cases we see, even without low-latency:

  1. Automatically "right sizing" a live setback that minimizes stalling with the optimal glass-to-glass
  2. Maintain synchronized live edge for shared watch events

Both of these use cases prefer that live adjustment can be turned off when a seek (or fast playback) moves the target position away from the minimal glass-to-glass time live point, perhaps we can talk about either automating this (we are using calls to LivePlaybackSpeedControl.setTargetLiveOffsetOverrideUs() to infer this now)

Anyway I can help out please ask here.

@marcbaechinger
Copy link
Contributor

Thanks Steve for the background! I commented in the PR. I think we can provide this with a bit less duplication before I go into the internal review with your change!

BTW: If you are ok with the suggestion I can apply this internally (still keeping your credits for the PR ofc). No need for you to update the pull request again. Would appreciate that you test this with your app first though to make sure the suggested change is equivalent to yours.

stevemayhew added a commit to TiVo/ExoPlayer that referenced this issue Mar 17, 2023
Any calls that issue a `seekTo()` where the position is `TIME_UNSET` will
now restore the live offset target to the MediaItem set default.

The code handles an intra-period seek in the same timeline by simply updating or removing
the `setTargetLiveOffsetOverrideUs()`.  Other cases call the original code.

This fixes issue google#11050
stevemayhew added a commit to TiVo/ExoPlayer that referenced this issue Mar 17, 2023
Any calls that issue a `seekTo()` where the position is `TIME_UNSET` will
now restore the live offset target to the MediaItem set default.

The change simply passes a flag to `updatePlaybackSpeedSettingsForNewPeriod()` that
indicates the call is from a seek operation.  If this flag is set, a `positionForTargetOffsetOverrideUs`
of `TIME_UNSET` unconditionally removes the `setTargetLiveOffsetOverrideUs()`

This fixes issue google#11050
@stevemayhew
Copy link
Contributor Author

stevemayhew commented Mar 17, 2023

Thanks @marcbaechinger. I tested the change and forced pushed the single commit. It works just the same (logic is same), but the code is cleaner all going through the same method.

I rebased before updating, so I picked up @tonihei 's fix for #10882 . I will back port his changes and this code to our 2.15.1 based release so we are running with the same code.

@stevemayhew
Copy link
Contributor Author

@marcbaechinger I'm looking at fixes for the initial seeks before prepare, ran across this bug. We have updated our fix to match what is in dev-v2 and both work, so I am closing this bug :-)

@google google locked and limited conversation to collaborators Oct 7, 2023
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

2 participants