-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
avoid call to HTMLMediaElement#load when peaks are given #1969
Conversation
calling `.load()` explicitly on a mediaelement entirely defeats the "preload" setting and makes it impossible to avoid downloading data before playback. When HTMLMediaElement#load is called, all of the major browsers make some request for the file. Chrome and FF make a range request for the entire file, but only report downloading them partially. Safari reports downloading the file in its entirety.
thanks @osheroff, could you check if there are any tickets that get fixed by this change (and add them to the PR description)? |
I would have guessed there would have been more.
There's also a whole host of people talking about memory issues etc, but they're likely using the webaudio backend. |
can you add a changelog entry? |
ps. it seems only Chrome dies during the CI builds, Firefox seems fine. We can disable Chrome on CI for a while and try again when it's back up and stable. Not being able to trust the CI results is annoying. |
Hi maybe the check can be done not if |
@marizuccara I think it's a good point, but maybe it should be both |
any further comments here? @marizuccara I modified it to check if |
branch has merge conflict. |
Ok I fixed the merge conflict. The system of putting changelog entries into PRs is a bit problematic, as every time another PR is merged into master my branch goes back into conflict (this is like the 3rd time I've had to update CHANGES.md on this PR). I'm wondering if there's a better solution... |
We can use a section like this at top to reduce merge conflicts:
Made that change in this branch. |
PR #1969 removed the call to MediaElement#load, but this means that we weren't properly triggering the waveform to render in the case where peaks were provided.
* add a call to drawBuffer in `loadMediaElement` PR #1969 removed the call to MediaElement#load, but this means that we weren't properly triggering the waveform to render in the case where peaks were provided. * update changelog entry * update comment, changelog
) * avoid call to HTMLMediaElement#load when peaks are given calling `.load()` explicitly on a mediaelement entirely defeats the "preload" setting and makes it impossible to avoid downloading data before playback. When HTMLMediaElement#load is called, all of the major browsers make some request for the file. Chrome and FF make a range request for the entire file, but only report downloading them partially. Safari reports downloading the file in its entirety. * code review; only avoid .load call when preload == 'none' * add changelog entry * bump changelog entry to next release * add next section to changelog Co-authored-by: Thijs Triemstra <info@collab.nl>
* add a call to drawBuffer in `loadMediaElement` PR katspaugh#1969 removed the call to MediaElement#load, but this means that we weren't properly triggering the waveform to render in the case where peaks were provided. * update changelog entry * update comment, changelog
calling
.load()
explicitly on a mediaelement entirely defeats the "preload" setting and makes it impossible to avoid downloading data before playback, which is often what the user is trying to do by providing peak data.When HTMLMediaElement#load is called, all of the major browsers make some request for the file.
Chrome and FF make a range request for the entire file, but only report downloading them partially. Safari reports downloading the file in its entirety.