audio-speaker icon indicating copy to clipboard operation
audio-speaker copied to clipboard

2.0 API proposal

Open dy opened this issue 7 years ago • 13 comments

Ideally API should be

const Speaker = require('audio-speaker')
let write = Speaker(options)
write(audioBuffer|data, callback?)

Stream counterparts are audio-speaker-stream and pull-audio-speaker

dy avatar Jun 16 '17 16:06 dy

The issue regarding the proposal for 2.0 is as follows.

We can either run with the current version of audio-mpg123 which is basically ready for release, just need to split up the projects of course.

Or we could wait until the NAPI version of audio-mpg123 is complete. Then make changes the API to follow the changes in audio-mpg123 then release.

Although the issue with releasing now is, the changes that will come with updating to the NAPI audio-mpg123 would require us to modify the API a bit. I suggest we could deprecate the methods and add newer methods for it that work better with ArrayBuffers for instance.

The issue with waiting for the NAPI version is that there would be a fairly long wait until NAPI is stable and ready to be used on production. To use NAPI modules they require the NAPI flag to add the dependency. Which would strike as an issue when installing audio-speaker. They will most likely remove this flag later on when it is stable.

So I need opinions on what way we should take this. @dfcreative @jamen 😄

vectrixdevelops avatar Jun 16 '17 23:06 vectrixdevelops

Hey. So I'm all up for using nan instead of node-addon-api if we can get something npm install-able out there faster. My only concern is I just don't want it to feel like a wasted effort, because NAPI may not be unstable for that long, and there is still things to do on the NAPI branch and here in the meantime that could close the gap.

I'll let you make the decision on where to invest your time, though.

jamen avatar Jun 16 '17 23:06 jamen

As for me I would choose getting things done rather than perfectly undone. That is if we can release this API within one-day effort but with nan that is perfectly fine, rather than if we start using node-addon-api and get stuck for months due to some unimplemented/unstable feature.

But we have always to be open to refactoring, ie. breaking API later to allow for node-addon-api is a good change.

dy avatar Jun 17 '17 02:06 dy

Cool, I'll get it released for nan then.

vectrixdevelops avatar Jun 17 '17 04:06 vectrixdevelops

Do we still want the browser implementation in 2.0? @dfcreative @jamen

vectrixdevelops avatar Jun 21 '17 21:06 vectrixdevelops

@connorhartley yes, but I can take care of it

dy avatar Jun 21 '17 21:06 dy

@dfcreative If you would like to make some tests for it that would be sweet! I am working in the release-2.0-browser branch, should be nearly ready. 😄

vectrixdevelops avatar Jun 21 '17 21:06 vectrixdevelops

@connorhartley I've added minimal easy-peasy test for browser and covered browser implementation here https://github.com/audiojs/audio-speaker/commit/944f25499e777e4082583700fbbef9b326becc05. Working on audio-oscillator by your request now.

dy avatar Jun 21 '17 22:06 dy

Thank you very much @dfcreative !

vectrixdevelops avatar Jun 21 '17 22:06 vectrixdevelops

Any chance I can test this out? I'm building an Electron app w/ node for streaming Amazon Polly speech mp3's

iwasrobbed avatar Jul 31 '17 05:07 iwasrobbed

@iwasrobbed That sounds awesome, feel free to keep us updated!

There is a pull request open at #44 where most of this is implemented already, if I'm not mistaken. I think the concern at the moment is how stable #44 actually is, I think we need more extensive testing, but I'm unable to check this at the moment (more could have been added since I've looked at it)

@connorhartley @dfcreative, what else needs to be done to wrap this up? :smile:

(P.S., if you are decoding with audio.js too, you might also want to see https://github.com/audiojs/audio-decode/issues/2)

jamen avatar Jul 31 '17 05:07 jamen

Thanks @jamen ; I gave it a quick test drive (Mac OS X Sierra), but doesn't seem quite ready yet:

Uncaught Error: Cannot find module '/Users/rob/blah/node_modules/audio-mpg123/lib/audio_mpg123.node'

Let me know if I can help test when you're ready and happy to do so

iwasrobbed avatar Jul 31 '17 06:07 iwasrobbed

@iwasrobbed We haven't added a prebuilt macos version to the releases as we have been struggling to find someone with macos to help us build it. Did it attempt to build it manually? Were there any errors involved with compiling the binding? If this continues, do you think you could open a separate issue for this? Thanks for your help. 😄

vectrixdevelops avatar Aug 01 '17 20:08 vectrixdevelops