search

Richienb / audic

Play some audio.
MIT License
68 stars 9 forks source link

Add 'ended' Event, add async setSrc method #11

Closed TilenGombac closed 2 years ago

TilenGombac commented 3 years ago

Resolves #8 while taking into account https://github.com/Richienb/audic/pull/9#pullrequestreview-627520949

also adds async setSrc method to allow awaiting loading of files into vlc

rejas commented 2 years ago

Maybe @Richienb could merge and release this? Or is something missing?

Richienb commented 2 years ago

The problem with all 3 pull requests is that I don't really want to deviate from the Web Audio API by adding a setSrc or setVlcInput method.

rejas commented 2 years ago

It is just an addition in my opinion, the "normal" API is still there. And for me the async setSrc is something usefull (or do you know a way for doing an async setter for the src?)

Richienb commented 2 years ago

It is just an addition in my opinion, the "normal" API is still there. And for me the async setSrc is something usefull (or do you know a way for doing an async setter for the src?)

We can probably create an async iife within the setter and check for its resolution in dependent functions.

rejas commented 2 years ago

You should write "I" in that sentence because I dont get it :-)

Richienb commented 2 years ago

How do we wait for the audio to start playing with the Web Audio API? Now let's replicate that.

TilenGombac commented 2 years ago

Why, if you don't mind me asking, are we trying to follow the Web Audio API spec to the letter, when as is, the current Audic implementation does not work like the Web Audio API does?

Not to mention that there's already the destroy() method, which does not exist in the Web Audio API.

We could of course check if it's been loaded when play() is called and await it there, we could emit the 'canplaythrough' event, etc... but it seems to me that this would all just make Audic more difficult to use, when a simple setSrc() method solves all of the issues that currently exist with Audic, while still keeping the API that WebAudio has.

Richienb commented 2 years ago

The Audio API is an excellent baseline to base a new API off of. I want to provide familiarity with it while also keeping it minimal.

The reason why I haven't added the other API methods is because I haven't added the other API methods. So technically this library is not complete yet and releasing it as a major version was a nieve decision.

The destroy() method is where I draw the line between spec consistency and utility. It is an essential function to avoid memory leaks but would seem awkward to call it remove(). Maybe that could be changed later on.

TilenGombac commented 2 years ago

The destroy() method is where I draw the line between spec consistency and utility. It is an essential function to avoid memory leaks but would seem awkward to call it remove(). Maybe that could be changed later on.

Well I'm glad the "we" finally turned into an I. I wish you the best of luck, but as the commits in this pull request are really all that I need (and it seems a lot of the users of your repository need), I'm just going to be publishing my fork and using that, and if you someday decide to implement the desired functionality in your desired way instead of telling others what and how to do it, I'll be glad to return to using this repo :)

rejas commented 2 years ago

I agree with @TilenGombac so I will use his fork too.