startingTime per track

[danielr18]

I'm going to be implementing a feature that will have to set a different startingTime per track, basically as long as you keep a track in the playlist but play another one, the time you were in is stored so that it can resume from there when you play that track again.

I wonder if this is something you would like to have in cassette, since you already have some logic for that, with props.startingTime and when you set this.media.currentTime = this.state.currentTime. The current props.startingTime isn't useful in this case since it's only used in the constructor.

[benwiley4000]

Should it go in the state snapshot if you move away from a track before it's finished? And then you could have a prop for whether you use that or not. And maybe a timeout prop for the amount of time you let a track play before you start saving its lastCurrentTime (so that way if you skip through a bunch of tracks you don't start all of them later at 0:00.234)? The timeout prop could have the same default value as stayOnBackSkipThreshold.

[danielr18]

I already have the logic that adds the currentTime in the playlist track after the actions that involve changing the active track, I think the playlist is a nice place to store the startingTimes, because the snapshot is only for the activeTrack and restoring snapshots is only done at the constructor.

I thought that maybe you can leave the logic to the users on how they provide the startingTime, but making sure that if the track has it, it plays from there.

[benwiley4000]

That probably makes enough sense for what you're doing, but for Cassette I would rather not build any features that modify playlist contents, since it relies on the user saving their updated playlist to replace the previous one.

What I was imagining besides just the snapshot is an internal state object that maps track urls to lastCurrentTime values. Every time currentTime updates, we update the object with the currentTime of the current track and include that object in the snapshot that gets saved (assuming we've passed the timeout I mentioned earlier). Every time a track finishes naturally, it gets removed from that object. Every time we construct a new PlayerContextProvider with a state snapshot, the object gets restored into state.

[benwiley4000]

I think I get why you want to store them in the playlist - it's because you have data on the backend on the last currentTime and they might not have had that track loaded in the last playlist, and it could make the state snapshot really huge over time if you're skipping away from a lot of different tracks.

What API did you have in mind? I don't think it makes sense to modify the playlist live, but maybe there's a hook we need for handling currentTime updates.

[benwiley4000]

Sorry just re-read:

I thought that maybe you can leave the logic to the users on how they provide the startingTime, but making sure that if the track has it, it plays from there.

You don't think there should be a "blessed" way to get updates on the starting time? I'm ok if not, just thinking about it.

[danielr18]

That probably makes enough sense for what you're doing, but for Cassette I would rather not build any features that modify playlist contents, since it relies on the user saving their updated playlist to replace the previous one.

What I was imagining besides just the snapshot is an internal state object that maps track urls to lastCurrentTime values. Every time currentTime updates, we update the object with the currentTime of the current track and include that object in the snapshot that gets saved (assuming we've passed the timeout I mentioned earlier). Every time a track finishes naturally, it gets removed from that object. Every time we construct a new PlayerContextProvider with a state snapshot, the object gets restored into state.

In my case with SSR, I can't even use snapshots, since I have to get the snapshot after the component mounts (to avoid differences on what's rendered), if I created the PlayerContextProvider after the app mounts, I might be able to use it, but all the other components in the app rely on the context provider being in the tree, so at the end I didn't end up passing the snapshot, I read the current time and set it manually to media element.

And if we were to keep a map of the current times, you would have to keep in sync with the playlist depending on what the user wants, some would like to keep the current time even if track is removed, that's not my case though, that's why I think it might not be necessary for cassette to handle how these are stored, and leave it to the user, since he's the one that handles everything that is related to the playlist.

[danielr18]

I don't have an issue modifying the playlist since doing so just run the checks of componentDidUpdate and getDerivedStateFromProps, which already run pretty frequently when you update the currentTime in state. However since playlist should be immutable, if you were to update the playlist every time you receive a time update, you would have the overhead of creating the new playlist, spreading the arrays and so... that's why I only update the playlist when the active track changes, we already get the currentTime with the snapshot handler, and I don't modify the playlist in there, currently store it in local storage, but could send to an API as well. There's no need to modify the startingTime in the playlist until you change the track in my opinion.

[danielr18]

Sorry just re-read:

I thought that maybe you can leave the logic to the users on how they provide the startingTime, but making sure that if the track has it, it plays from there.

You don't think there should be a "blessed" way to get updates on the starting time? I'm ok if not, just thinking about it.

I've been doing fine getting it from onStateSnapshot. What else would you do different in a "blessed" way?

[danielr18]

I think I get why you want to store them in the playlist - it's because you have data on the backend on the last currentTime and they might not have had that track loaded in the last playlist, and it could make the state snapshot really huge over time if you're skipping away from a lot of different tracks.

What API did you have in mind? I don't think it makes sense to modify the playlist live, but maybe there's a hook we need for handling currentTime updates.

I don't have nothing conclusive yet, but something that lives in the track object, similar to the duration wouldn't be too bad from my pov.

[benwiley4000]

In my case with SSR, I can't even use snapshots, since I have to get the snapshot after the component mounts

So I was assuming you were using snapshots after the discussion in https://github.com/benwiley4000/cassette/issues/318. Did something change? It sounded to me like storing a single track in your cookies to initialize the playlist on the server side was sufficient. If that didn't solve your use case I'd like to come back to that issue (I know it's sort of off-topic here but I'd like to come up with a good solution).

I've been doing fine getting it from onStateSnapshot. What else would you do different in a "blessed" way?

Well I intentionally didn't document the insides of that object and stuck it under an __unstable__ key so no one would try to read anything inside of it. 😉 I might change it in a non-major release (in a way that older snapshots would auto-upgrade). Maybe I need to make the __unstable__ key longer and more annoying to type.. 😄

I don't have nothing conclusive yet, but something that lives in the track object, similar to the duration wouldn't be too bad from my pov.

Makes sense. Probably best to support the same form for both.

[benwiley4000]

There's no need to modify the startingTime in the playlist until you change the track in my opinion.

The only use case I thought of was:

1. I am playing a track and my browser crashes/I leave the page
2. I come back to the page later and have a different playlist at the start, without the track I was playing before
3. I end up adding that track to the playlist and when I get to it, I want it to resume where I left off

Of course, as you mentioned just now, you aren't looking for this feature. So I agree that for your case it would make sense to only check when the active track changes.

[danielr18]

In my case with SSR, I can't even use snapshots, since I have to get the snapshot after the component mounts

So I was assuming you were using snapshots after the discussion in #318. Did something change? It sounded to me like storing a single track in your cookies to initialize the playlist on the server side was sufficient. If that didn't solve your use case I'd like to come back to that issue (I know it's sort of off-topic here but I'd like to come up with a good solution).

I ended up dropping the cookies, it made the whole thing too complicated to only get the player view from the server render, and at the end it wouldn't be interactive until React hydrates, so nothing much to gain. The reason it got complicated is that we have something called placeholder tracks, basically if you don't have any track in the playlist, we set the first episode you see in the page you are, and until you play that episode or another one, the placeholder track is changed when you move between pages, also when the last track finishes (and it's the only one left), it becomes placeholder, so it gets changed when you navigate to another page.

The placeholder track logic made the storage in cookie complicated at the end.

I've been doing fine getting it from onStateSnapshot. What else would you do different in a "blessed" way?

Well I intentionally didn't document the insides of that object and stuck it under an __unstable__ key so no one would try to read anything inside of it. 😉 I might change it in a non-major release (in a way that older snapshots would auto-upgrade). Maybe I need to make the __unstable__ key longer and more annoying to type.. 😄

Haha, something like dangerouslySetInnerHTML, sorry, I can always subscribe to the timeupdate event, but if you want to be able to pass a handler as a prop, I'm cool with that.

I don't have nothing conclusive yet, but something that lives in the track object, similar to the duration wouldn't be too bad from my pov.

Makes sense. Probably best to support the same form for both.

[danielr18]

There's no need to modify the startingTime in the playlist until you change the track in my opinion.

The only use case I thought of was:

1. I am playing a track and my browser crashes/I leave the page
2. I come back to the page later and have a different playlist at the start, without the track I was playing before
3. I end up adding that track to the playlist and when I get to it, I want it to resume where I left off

Of course, as you mentioned just now, you aren't looking for this feature. So I agree that for your case it would make sense to only check when the active track changes.

Yes, you could always store the map for all tracks' currentTimes yourself in local storage or in a database, and when you add the track back, you check if you have it stored, and add it to the playlist track object before adding it to the playlist.

[danielr18]

I think that it would be easier for cassette to just deal with handling a track with a startingTime and a way to subscribe to the currentTime change as you play a track, like the timeupdate event, so you can store it in any way you like for the use case you want to have.

[benwiley4000]

The placeholder track logic made the storage in cookie complicated at the end.

Cool... I'm willing to revisit adding a way to restore state from snapshot after component mount, if you have an idea for the API. I didn't see a great reason for it originally, but I'm not trying to make your life difficult. 😆

Haha, something like dangerouslySetInnerHTML, sorry

Well my intention was more like __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED lol... (although to be transparent/fair I started using that in the hooks package but as you can see I'm being extra careful I don't break anything if the API changes).

I think that it would be easy for cassette to just deal with handling a track with a startingTime and a way to get the currentTime as you play a track, so you can store it in any way you like for the use case you want to have.

Yeah, I can see now that's the least-complicated thing to do and we don't need a silver bullet. An onTimeUpdate method could work? And we could pass it the current track and index, just like onActiveTrackUpdate - just to avoid confusion over what the current track is.

[danielr18]

The placeholder track logic made the storage in cookie complicated at the end.

Cool... I'm willing to revisit adding a way to restore state from snapshot after component mount, if you have an idea for the API. I didn't see a great reason for it originally, but I'm not trying to make your life difficult. 😆

I haven't found the need yet to use the snapshot other than to restore currentTime, and that could be solved with this solution. However, we might think of a solution to restore snapshots after component mounts later on.

Haha, something like dangerouslySetInnerHTML, sorry

Well my intention was more like __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED lol... (although to be transparent/fair I started using that in the hooks package but as you can see I'm being extra careful I don't break anything if the API changes).

Haha, yeah, that one came to mind as well.

I think that it would be easy for cassette to just deal with handling a track with a startingTime and a way to get the currentTime as you play a track, so you can store it in any way you like for the use case you want to have.

Yeah, I can see now that's the least-complicated thing to do and we don't need a silver bullet. An onTimeUpdate method could work? And we could pass it the current track and index, just like onActiveTrackUpdate - just to avoid confusion over what the current track is.

This makes sense.

[benwiley4000]

Did not mean to close lol. Did you want to make a PR?

[danielr18]

Yes, I will

[benwiley4000]

Thanks for bearing with me while we thought this one through!

[benwiley4000]

I haven't found the need yet to use the snapshot other than to restore currentTime, and that could be solved with this solution. However, we might think of a solution to restore snapshots after component mounts later on.

Does this mean you don't want to restore other state like muted/volume? Or is it more that you need a way to filter out things you don't want to restore?

[danielr18]

It's because I'm not using those yet, but I might in the future, so we can open an issue for it later, since I'd need a way to restore snapshot after it mounts

[danielr18]

In order to set currentTime correctly on iOS, you have to set it after the canplaythrough event. See here: https://stackoverflow.com/questions/18266437/html5-video-currenttime-not-setting-properly-on-iphone. Would you be ok with changing when is trackLoading set to false?

During the loading process of an audio/video, the following events occur, in this order:

loadstart durationchange loadedmetadata loadeddata progress canplay canplaythrough

I think it makes sense to do it in handleMediaCanplaythrough instead of handleMediaLoadedmetadata, since that's when it can start playing and the loading ends.

The event handlers would end up like this (dist code, sorry):

_proto.handleMediaLoadedmetadata = function handleMediaLoadedmetadata() {
  if (this.state.trackLoading && this.media.currentTime !== this.state.currentTime) {
    // correct currentTime to preset, if applicable, during load
    this.media.currentTime = this.state.currentTime;
  }
};

_proto.handleMediaCanplaythrough = function handleMediaCanplaythrough() {
  if (this.state.trackLoading && this.media.currentTime !== this.state.currentTime) {
    // correct currentTime to preset, if applicable, during load
    this.media.currentTime = this.state.currentTime;
  }

  this.setState(function (state) {
    return state.stalled === false && state.trackLoading === false ? null : {
      stalled: false,
      trackLoading: false
    };
  });
};

We set the currentTime in the metadata event as well, since that happens faster, and it works well on other platforms with the exception of iOS.

[benwiley4000]

Notice the comment from stackoverflow: https://stackoverflow.com/questions/18266437/html5-video-currenttime-not-setting-properly-on-iphone#comment75364746_18283198

This is mostly right except that you can listen to the loadedmetadata event instead of canplay since it's fired earlier. – bfred.it May 25 '17 at 7:40

That was what I thought, and I knew this sounded familiar. You can see here we used to set currentTime immediately but now we wait until loadedmetadata. https://github.com/benwiley4000/cassette/blob/next/packages/core/src/PlayerContextProvider.js#L226

I also see this answer which suggests loadeddata (in case loadedmetadata really doesn't work?): https://stackoverflow.com/a/50951559/4956731

So I'm a bit confused. Are you sure it needs to wait until canplaythrough? That will be a super long time for a long clip I think?

[danielr18]

I can tell you that it doesn't work in loadedmetadata for iOS, when it starts to play, it starts from 0;. Let me check if another event before canplaythroughworks.

[danielr18]

loadeddata works as well.

[danielr18]

What's your opinion on trackLoading?

[benwiley4000]

Is it possible to check immediately after setting currentTime if it worked or not? That way you could set track loading false if it did work, and wait until loadeddata if it didn't?

[danielr18]

The value after you set it is the one that you set, but then at some point between loadedmetadata and play, it's reset to 0.

Even if there wasn't this issue with currentTime, there's still a period between loadedmetadata and the track playing, wouldn't it be better to stop showing a loading indicator in the UI, when the audio can start playing (by setting trackLoading: false in canplaythrough)?

[benwiley4000]

Yeah I see your point. You're probably right.

My main question is about this line: https://github.com/benwiley4000/cassette/blob/next/packages/core/src/PlayerContextProvider.js#L677

As long as the track is still loading we prevent any time updates from happening. Is there any scenario where a timeupdate would happen between metadataloaded and dataloaded and we would cause problems? I would suspect not.. as long as we're good there, I think your proposed change makes sense. We can probably just change the metadataloaded handler to a dataloaded handler?

[benwiley4000]

I looked and it seems like no that will never happen, so it makes sense to listen to loadeddata instead of loadedmetadata.