Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Deprecate playAt for seeking initial position #211

Merged
merged 8 commits into from
Apr 4, 2019
Merged

Conversation

Mecharyry
Copy link
Contributor

@Mecharyry Mecharyry commented Apr 4, 2019

Problem

Whilst working on the adverts loader I noticed that when we want to perform a seek to an initial position we do:

loadVideo -> playAt / seekTo

This has very little chance of notifying us in the correct order. ExoPlayer is essentially message based so you are going to be rendering the first frame of content before performing a seek in this flow. After looking at the ExoPlayer code in a little more detail I can see that we need to perform a seek before we prepare the underlying player. This way the timeline and the position information is updated prior to the first frame being rendered.

Solution

Add an initialPositionInMillis to the Options. When this Option is available we perform a seek and tell the player not to reset the positional information for the player. This means when the first frame rendered it will be from that position in the Timeline.

Test(s) added

Just updating the facade tests to ensure that when this position information is present we will perform a seek before playing.

Screenshots

For those that are interested I have some rather long logs. These are informational, so these are the listeners that exo-player calls when it does anything

Before

caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onLoadStarted
caller: onLoadStarted
caller: onSurfaceSizeChanged
caller: onLoadCompleted
caller: onLoadCompleted
caller: onTimelineChanged
caller: onTimelineChanged
caller: onMediaPeriodCreated
caller: onMediaPeriodCreated
caller: onLoadingChanged
caller: onLoadingChanged
caller: onDecoderEnabled
caller: onDecoderEnabled
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onTracksChanged
caller: onTracksChanged
caller: onReadingStarted
caller: onReadingStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onDownstreamFormatChanged
caller: onDownstreamFormatChanged
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onDownstreamFormatChanged
caller: onDownstreamFormatChanged
caller: onBandwidthEstimate
caller: onDecoderInitialized
caller: onDecoderInputFormatChanged
caller: onBandwidthEstimate
caller: onDecoderInitialized
caller: onDecoderInputFormatChanged
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onVideoSizeChanged
caller: onRenderedFirstFrame // RENDERS FRAME :100:
caller: onSurfaceSizeChanged
caller: onAudioSessionId
caller: onPlayerStateChanged
caller: onSeekStarted   // POSITION INFORMATION CORRECT AFTER THIS POINT
caller: onPositionDiscontinuity
caller: onPositionDiscontinuity
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onSeekProcessed
caller: onPositionDiscontinuity
caller: onBandwidthEstimate
caller: onLoadCanceled
caller: onLoadCanceled
caller: onBandwidthEstimate
caller: onLoadCanceled
caller: onLoadCanceled
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onRenderedFirstFrame  // RENDERS IT AGAIN?!!! :x:
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onSurfaceSizeChanged

After

caller: onSeekStarted // POSITION INFORMATION CORRECT AFTER THIS POINT
caller: onPositionDiscontinuity
caller: onPositionDiscontinuity
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onLoadStarted
caller: onLoadStarted
caller: onSurfaceSizeChanged
caller: onLoadCompleted
caller: onLoadCompleted
caller: onTimelineChanged
caller: onTimelineChanged
caller: onSeekProcessed
caller: onPositionDiscontinuity
caller: onMediaPeriodCreated
caller: onMediaPeriodCreated
caller: onLoadingChanged
caller: onLoadingChanged
caller: onDecoderEnabled
caller: onDecoderEnabled
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onLoadStarted
caller: onTracksChanged
caller: onTracksChanged
caller: onReadingStarted
caller: onReadingStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onDownstreamFormatChanged
caller: onDownstreamFormatChanged
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onDownstreamFormatChanged
caller: onDownstreamFormatChanged
caller: onBandwidthEstimate
caller: onDecoderInitialized
caller: onDecoderInputFormatChanged
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onDecoderInitialized
caller: onDecoderInputFormatChanged
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onAudioSessionId
caller: onLoadStarted
caller: onLoadStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onLoadStarted
caller: onLoadStarted
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted
caller: onVideoSizeChanged
caller: onSurfaceSizeChanged
caller: onRenderedFirstFrame  // RENDERS ONCE!
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onPlayerStateChanged
caller: onBandwidthEstimate
caller: onLoadCompleted
caller: onLoadCompleted

Paired with

Nobody.

@Mecharyry Mecharyry added the bug label Apr 4, 2019
@Mecharyry
Copy link
Contributor Author

test this please

boolean hasInitialPosition = options.getInitialPositionInMillis().isPresent();
if (hasInitialPosition) {
Long initialPositionInMillis = options.getInitialPositionInMillis().get();
exoPlayer.seekTo(initialPositionInMillis);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is the fix

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, sorry @juankysoriano I should have commented this line 😄

@xrigau xrigau merged commit 39fe9c2 into develop Apr 4, 2019
@xrigau xrigau deleted the initial_position branch April 4, 2019 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants