benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Setting media.currentTime >= duration loads complete file #329

Open danielr18 opened 5 years ago

danielr18 commented 5 years ago

This doesn't seem to be an specific issue of cassette but with the HTML5 video/audio implementation in Chrome (where I'm testing) at least.

I created a minimal reproduction: https://codesandbox.io/s/lx18nkkoll. You can try using "Seek to duration" button and check in the network tab that it will begin downloading the whole file, whereas if you click "Seek to math.floor(duration)" button, the track will end immediately, as one would expect. See if you can recreate it.

Should cassette prevent this behaviour and if so, where do you think it's more appropriate to handle it?

benwiley4000 commented 5 years ago

Hmm, interesting problem! What would you suggest a fix would look like? And wouldn't floor mean that the seek would be inexact?

I'm not in front of my computer but I'm wondering if this problem also exists in other browsers, and if the HTML spec says anything about it.

danielr18 commented 5 years ago

Firefox on MacOS doesn't seem to have the issue.

benwiley4000 commented 5 years ago

Thanks again for flagging this. I'll look into it later.. might be a Chrome bug.

benwiley4000 commented 5 years ago

I just reproduced this issue, and I think I understand what's happening.

In Chrome (and in Firefox), when we seek to a new timestamp, under normal circumstances we load just a bit of the file to allow smooth playback for the next 30 seconds or so. For smaller files, this might be the whole thing, but for really long tracks like your podcasts, this will be a tiny percentage of the total length of the file.

But in Chrome as you've noted, the behavior is weird when we seek to a timestamp greater than or equal to the duration of the track. Chrome loads the entire segment of the track between the previous currentTime and the end of the track. So in your case you found that it loaded the whole track, but when I seek to the halfway point first and then to the end, it only loads half of the track.

However if that range is already loaded, then nothing will be loaded when you seek to the end. e.g. if you hit the "Seek to math.floor(duration)" button first and then the "Seek to duration" button, there won't be anything new to load between the previous currentTime and the end of the track, so you won't see any new events in the network tab. This behavior is normal and expected, I'm just pointing it out so we don't get confused when we can't trigger the bug multiple times without refreshing.

I'd like to have a minimal repro for this demonstrating how different ranges are loaded depending on the starting timestamp (so we can file a bug in Chromium). It might be useful to have multiple long audio files for this. Do you have any more media links you can share for a repro? Or if you'd like to prepare the repro yourself, I wouldn't mind 😄 . Whatever you have time for!

benwiley4000 commented 5 years ago

Actually, I think one file is enough for now. I'll work on the ticket for Chromium!

danielr18 commented 5 years ago

I think your hypothesis is correct, I can sort of recreate that behaviour. If you need the files at the end, just let me know.

benwiley4000 commented 5 years ago

So I went through the effort to create a minimal repro here in jsfiddle: http://jsfiddle.net/bqhy156n/22/

And... I don't have the same issue anymore. Do you think I missed anything with the repro?

I tried looking through the Cassette code to figure out what might be wrong. I figured maybe something's not right in the code that's run on the ended media event, but I didn't have much luck going down that path. It seems like even if I cancel that event handler we still have the track load in the repro example.

If you feel like poking around, feel free! I'll come back to this later if it's still unsolved.

danielr18 commented 5 years ago

I don't have the issue either on that repro, I'll see if I can get it to reproduce tomorrow.

benwiley4000 commented 5 years ago

Just kidding, I got it to repro: http://jsfiddle.net/bqhy156n/26/

I had to add step="any" to the range input. Then it outputs decimal numbers and the reload is triggered. Otherwise it comes out with round numbers (like your "workaround" from before).

benwiley4000 commented 5 years ago

@danielr18 ok so after listening to this whole thing... actually could I have a link to a different podcast episode so the chrome developers don't think I'm insane? 😄

danielr18 commented 5 years ago

© Copyright 2012-2018 NPR - For Personal Use Only

© Copyright 2019 Serial Podcast

benwiley4000 commented 5 years ago

Serial is great! I'll use that ☺️

benwiley4000 commented 5 years ago

https://bugs.chromium.org/p/chromium/issues/detail?id=919328

I'll leave the issue open for now since it's a typical problem that could happen with this library.