Support For Providing A StartPosition With Track

[thorbenprimke]

Summary:
This adds support for providing a startPosition with a track. If a startPosition
is provided, the track will start playing from the specified position.
The position is in seconds.

I chose to name it startPosition because we may want to add support for providing
an endPosition as well in order to only play a sub-clip of a track. It’s not part of this
PR as it requires additional changes to SwiftAudio on the iOS side. ExoPlayer on the
Android side already supports this.

Test Plan:

• verified on both iOS and Android with the example app
• used a new sound file that counts in order to easily verify that startPosition works as expected

[curiousdustin]

When I try this, on Android, it appears to affect the reported duration and position values such that the duration is the "clipped" duration instead of the entire duration.

For instance, if I have a track that has a total duration of 0:30, and I want to start at 0:10, the track starts and plays as expected at 0:10, however, the duration is reported as 0:20, and the position is 0:00 at the start.

This means that progress indicators and timeline seek controls are affected.

Is this what you intended, and found? It is different than the iOS behavior.

[curiousdustin]

Maybe you had a different use case in mind? My intended use case for this feature would be to "resume" a track at a given time. Perhaps where a user left off in a previous listening sessions.

[biomancer]

Yeah, it seems that clipping does not fit here - but I've found an example of setting initial position in expoplayer demo - https://github.com/google/ExoPlayer/blob/r2.10.4/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java#L447 - example just uses seekTo before setting a media source, probably that's the way?

[curiousdustin]

I think that is probably the correct way as well. However, I don't feel I have a good enough grasp on the Android native code to suggest an exact implementation. :(

[curiousdustin]

I can see multiple places that would work for adding this on "play". But I'm not seeing an obvious place to do a seek when the queue goes from one track to the next. In order to match the iOS implementation, you should be able to queue up multiple tracks, each with a start time, and have each of them start at the specified time.

Does prepare() get called when transitioning to the next track?

[thorbenprimke]

I had only tested it in the example app and it seemed to do the right thing. But yes, it does cause a problem with the duration.

The duration that is passed into a track (which the example didn't do), doesn't match with the actual length due to how it does the clipping.

Yes, for the first item setting seekTo before prepare on Android works but for subsequent tracks, I think the onPositionDiscontinuity listener could possibly be used. Both DISCONTINUITY_REASON_PERIOD_TRANSITION and DISCONTINUITY_REASON_SEEK may provide enough information to execute the seekTo at the point. I'll dig into tonight to see if that ensure resume of any number of tracks in the queue works.

[thorbenprimke]

So using onPositionDiscontinuity and setting the startPosition/initialTime there works.

It works best for DISCONTINUITY_REASON_SEEK as the position of the audio is always 0 and thus seeking doesn't cause an audible disturbance. This case is for the first track in a queue as well as if one uses the move to next / previous button / command.

However for DISCONTINUITY_REASON_PERIOD_TRANSITION which happens when you transition from one track to another, it has already pre-loaded the next track and is starting to play it. Because of this I log values between 20-40 ms and thus when seek is initiated, it is audible that it goes from playing to seeking to playing.

To make this work perfect as that it actually sets the seek before playing, the exoplayer has to be modified. There is already the notion of a startPosition (different from the one of the ClippingMediaSource) but as far as I can tell, it has to be exposed in order to use it for the case of resuming a track at a specific seek. That's how far I got last night.

[curiousdustin]

@Guichaguri Do you have any guidance for us, as to how/where to implement a startPosition/initialTime in the Android?

[Guichaguri]

I'd love to hear about your use case. I'm still confused about this PR, I can see two distinct features being mixed together:

• Clipping/cutting the track (which is what you're doing on Android): Treats the track as it never had anything before the start position.
  • The duration changes, you can't seek to the start of the track, the track will always start from that position, like it was originally clipped.
• Starting to play from a specific position (the same effect from loading and then seeking on Android)
  • The duration remains the same, you can still seek to the start of the track.

In case you want the latter, the proper way is to seek to the start position before playing.

[Guichaguri]

Here's another question:

• Track 1 is added with a start position of 0:10
• Track 2 is also added
• Track 1 starts to play from the start position
• Track 1 ends and then Track 2 starts to play
• The user skips to the previous track

Should the Track 1 start to play from 0:10 or 0:00?

[gaodeng]

currently the queue is hard to use ,why don't we let the js manage the queue? for gapless playback, emit a about to finish event,then js append the next track to preloading

[curiousdustin]

I think the use case is number 2. At least I know mine is. I want to start playing at a given time in order to resume where a user left off previously.

The reason we are trying to implement this feature is: await play() then ‘await seekTo()` (or other forms of the same idea) doesn’t consistently work as expected on iOS. In certain cases the seek doesn’t happen because the native player is not in a state that allows it.

SwiftAudio provides the option to set an initialTime instead. As this PR shows. This works as expected every time.

Regarding skipping to previous track, no I would NOT want it to start at the specified time. I would expect it to start at zero. However, in my case this is irrelevant because I am managing my own queue and only have one track at a time in the native queue.

Maybe the better solution is some sort of playFromTime() method implemented across platforms? This would keep the track object as is, but allow you to effectively play and seek as a single action and not worry about the various thing that can go wrong if they are separate.

[Guichaguri]

@gaodeng The reason why we manage the queue is to allow gapless playback. We already plan to add a player that doesn't have a managed queue in v2, but that's a subject for another issue.

@curiousdustin I totally agree with you, I think play() should accept an optional time parameter, which makes it play from that time. Most players work that way, I don't think react-native-track-player should be different.

[curiousdustin]

@Guichaguri, leaving the alternate play(fromTime) plan aside for a moment, can you give us your thoughts on the effectiveness of doing the seek within onPositionDiscontinuity such as @thorbenprimke has done here: https://github.com/react-native-kit/react-native-track-player/pull/713/files/c8c260f8b80376c087ed9b9915533fd2abb2299d..147e216c30f3a389e54ee551d9caed6dd6cccd5b

Is this approach guaranteed to seek to the designated time without any chance of a tiny bit of audio playing before the seek happens?

Is onPositionDiscontinuity called in ALL cases when a new track is played?

[curiousdustin]

@Guichaguri , I would possibly start a PR for the play(fromTime) feature, but the 1.1.x branch is currently broken, making it hard to have a good branching off point to start from. #703

(I'm still on 1.1.x because I haven't updated react yet)

[thorbenprimke]

@curiousdustin - No, I don't think using onPositionDiscontinuity prevents a tiny bit of audio being played due to the nature of it being a callback. At least in my testing, audio is sometimes audible. This may be less of a case for prod builds but for debug builds with logging, it is noticeable.
In regards to your second question, I have found that it is called in the initial play, continued play as well as skipping cases. The logic that was already there detects if it's a track change.

@Guichaguri The goal of this PR was to add functionality that would allow providing an initialTime / initialSeek to any track. The use of ClippingMediaSource was incorrect (but this wasn't obvious at first with the example app).

The problem it addresses is that in a playlist, if more than one track was previously played, we need to set an initial seek not just for the first item in the playlist but also subsequent items.
A scenario is that someone has an item at position 0 in the playlist and it is played for x seconds. Now they change the order of the playlist and play a different track that is now at position 0 and is played for some time and stopped.
Now when the app is restarted, the playlist needs to be rebuilt and two tracks need an initial seek to properly resume the playing of each of the respective tracks.

• Track 1 is at position 0 and is played to 15 seconds
• Track 2 is added to position 1 and then moved to position 0
• Track 2 is played for 20 seconds
• Track 2 is paused and app closed.
• App is reopened
• Track 2 (in position 0) needs to have an initial seek of 20 seconds
• Track 1 (in position 1) needs to have an initial seek of 15 seconds when playback is transition from Track 2 to Track 1

To answer your question as to what should happen if the user skips from Track 1 back to Track 2 (in the above example), Track 2 would play again from the previous initial seek based on both using onPositionDiscontinuity (with ExoPlayer) as well as initialTime (with SwiftAudio).
In our case we don't allow going backwards in the playlist if a track has finished as it is immediately removed (thus I'm less concerned with this).
However what should be happening (in my opinion) is that the track needs to be updated when the user skips from Track 2 (let say at 30 seconds) to Track 1 (starts at 15 seconds) forward and thus when the skip backwards, the initial seek time is actually the last played time of Track 2 (thus should play at 30 seconds instead of 20 seconds).

Since I'm not 100% happy with using onPositionDiscontinuity, I explored a solution in ExoPlayer to support this and to get guidance as how it might be implemented best. The thread is here and suggestion based on ExoPlayer current architecture is to create a wrapping MediaSource to allow for the initial seek. However it is mentioned that they have discussed this internally and may add better support for this in the future.

[thorbenprimke]

@FrankGoortani - I appreciate the vote of confidence but I'm not certain that we should move forward with the proposed solution. I'm using it internally for the time being but the proper solution should be to address this on ExpPlayer's side first (by adding support for this).

[curiousdustin]

@thorbenprimke, did you end up finding or creating a solution that doesn't rely on onPositionDiscontinuity?

[curiousdustin]

For those interested, I ended up creating a custom media source wrapper that allows setting a start time. It probably doesn't apply to all use cases but it seems to work for mine.

It does not rely on onPositionDiscontinuity, so it does not suffer from playing the start of the track.

It also allows seeking backward from the set starting point.

I followed the example given here by the ExoPlayer team. Had to modify quite a bit because things have changed, but the spirit remains the same.
google/ExoPlayer#2403 (comment)

Here are the relevant changes on my fork:
curiousdustin@4e152e7

[tmaly1980]

So what's a workaround for this? I've tried calling seekTo() prior to calling play() and it plays from the start. If I call seekTo() after I call play() then it automatically pauses.

[jspizziri]

@dcvz @bradleyflood it looks like there's some movement here. Is there anything I can do to help out?

[jspizziri]

@dcvz @bradleyflood , sorry I'm not trying to be annoying, I greatly appreciate all your work on this project.

But I'm pleading with you here. I want to solve this problem for you. I just want some high-level consensus regarding the proper solution before I spend time working on it.

🥺 🥺 🙏

[dcvz]

I think here there's a big discontinuity between the two options for such an API.

1) providing an initial position parameter as part of the track.

However this has the problem that this would be taken into account anytime that track plays, for example if you skip forward and you skip backward. The track will just continue to start at the initial time set.

2) we have methods allowing you to play a track starting at a designated position.

For playing the first track, this should already be possible by adding items to the queue and seeking before calling play() but we'd likely have to update skipToNext(), skipToPrevious() and skip(index) to support passing in a time parameter at which they should start playing.

===

I personally think I'd be in favour of an approach like no. 2 instead of option 1. What do you think @mpivchev?

For the interested parties here, is option 2 one that would fit your use cases?

[jspizziri]

@dcvz thank you for responding! I really do appreciate it.

FWIW the following interface would satisfy my requirements (effectively "Option 2" as I understand it):

interface TrackPlayer {
  public async play();
  public async skipToNext(initialPosition?: number);
  public async skipToPrevious(initialPosition?: number);
  public async skip(index: number, initialPosition?: number);
}

Also FWIW, the scenario of calling seekTo prior to play is currently having some issues on iOS. But that's a separate issue and I agree that in theory that combined with the above interface solves our use-case.

[martinmidtsund]

Hi @dcvz ! Great work on this library :)

We are also in need of this functionality. Our use case is playing a queue of one or more tracks that has been previously started and then saved each tracks position.

This would fit option 1 more, I guess, as the queue should be able to run by itself and when a track ends, the next one should automatically start on the stored position for that track.

A patch from @gilbertl that uses the initialTime property for iOS makes our use case with option 1 doable with the current interface. As @jspizziri mentions, the seekTo before play works fine on Android, but not on iOS.

[jspizziri]

@martinmidtsund @dcvz ,

The way I see it "Option 1" could be layered on top of "Option 2". If that's the case and we all agree that Option 2 would be a useful set of functionality then perhaps we could implement it and implement the rest later?

[doughsay]

I'd like to add my use-case to the mix, which I think tends towards needing Option 1 (which I do understand is a bit problematic from a multi-track queue perspective). In my case, I only ever add one track to the queue. If the user switches tracks, I always call reset before adding the new track. From my testing, I see that as soon as you add a remote (hls/dash) track to the queue and it is the "active" track, track-player will start buffering the track from the server. Yes I can seek immediately after adding, but there's still a bit of a delay, so track-player always starts buffering from position zero, even if I immediately seek afterwards.

When I opened my issue a while back (#1330) I mentioned this "waste of bandwidth" as the main problem. It's honestly not really that big of a deal, but I just wanted to point it out in case it wasn't being considered.

My use-case is an audiobook app in which you only ever have one "active" book, and you tend to listen for a long period of time. Switching between books is when I reset the player and load in a different track, but I restore the previous position from the server. But I can see another argument for Option 1 being something like a podcast app, where you can add multiple "episodes" to your queue, but you still always want to restore the playback position, not start playing from the beginning.

Just my two cents! And thanks for this amazing library! ❤️

[jspizziri]

@dcvz any thoughts on the comments of the folks who have chimed in?

[dcvz]

Yeah, I've thought some more about it. I still think we should go with the option 2 approach. I think it still lends itself to being able to be used for the other use cases as well, albeit with a bit more work.

If you're up for that exploration @jspizziri lets do it! I think not being able to seek on iOS is a bug that should be fixed.

[jspizziri]

@dcvz I will put this in my schedule for next week, stay tuned!

[jspizziri]

Support for the discussed interface is largely functional on this PR. I've recommended that we close this PR in favor of the other.

With that said with the fix to iOS seek on load @dcvz is hesitant to merge it as this might be a non-issue at this point with a reasonable solution out of the box.

There's an ongoing conversation around theoretical vs. actual performance that could use the input from the people on this thread. If you all could chime in and give your feedback or give the PR a test it would be helpful to determine what the future of the PR will be.

[jspizziri]

#1413 has been merged, and I believe this feature will land in v2.2.0-rc4 (which at the time of this writing does not exist).

@dcvz I think this PR can be closed.