Skip to content
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

WaveSurfer load() should accept HTMLMediaElement when using WebAudio #2148

Merged
merged 4 commits into from
Jan 18, 2021

Conversation

sundayz
Copy link
Contributor

@sundayz sundayz commented Dec 30, 2020

The WaveSurfer.load function's documentation says that the url parameter can be a string or a HTMLMediaElement, but when using the WebAudio backend, the loadBuffer function will assume the URL is always a string, and send incorrect HTTP requests like this:

image

Breaking in the external API:

Technically this affects the external API, but instead of breaking existing code, it will fix currently broken code.

@sundayz sundayz added the bug label Dec 30, 2020
@coveralls
Copy link

coveralls commented Dec 30, 2020

Coverage Status

Coverage increased (+0.2%) to 82.584% when pulling f644a79 on sundayz:fix-webaudio-load into d134757 on katspaugh:master.

Copy link
Contributor

@thijstriemstra thijstriemstra left a comment

Choose a reason for hiding this comment

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

Looks good. Please add a test(s) as well.

@sundayz
Copy link
Contributor Author

sundayz commented Jan 18, 2021

I had to make a new file for WebAudio tests, let me know if it looks OK, I don't know how you'd like to structure the tests

Copy link
Contributor

@thijstriemstra thijstriemstra left a comment

Choose a reason for hiding this comment

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

please add a changelog entry, then squash and merge, thanks.

@sundayz sundayz merged commit 658bd92 into katspaugh:master Jan 18, 2021
sandiz pushed a commit to sandiz/wavesurfer.js that referenced this pull request Sep 1, 2021
…atspaugh#2148)

* Allow users to pass in HTMLMediaElement to the load() function when using WebAudio backend

* add webaudio tests

* add changelog entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants