audio-speaker
                                
                                 audio-speaker copied to clipboard
                                
                                    audio-speaker copied to clipboard
                            
                            
                            
                        2.0 - Custom Binding
This implements an entire remake of the current audio-speaker API with a custom binding. The aim is to make this a more maintained version for the AudioJS ecosystem. This uses the pre-built binding options first and if that does not exist for your OS version it will attempt to build it manually. This also contains a browser version that will be used if installed with the --no-optional flag.
This is a work in progress.
@connorhartley looks great, going to give it a try.
@connorhartley couple of moments.
First, is it possible to remove/reduce logs for binding compilation? There are some warnings, removing them would be awesome, but you know better.
Log
C:\projects\audio-speaker>npm install
> [email protected] install C:\projects\audio-speaker\node_modules\audio-mpg123
> node ./scripts/check-pre-gyp && node-pre-gyp install --fallback-to-build
node-pre-gyp ERR! Tried to download(404): https://github.com/audiojs/audio-mpg123/releases/download/1.2.3/audio_mpg123-v
1.2.3-node-v57-win32-x64.tar.gz
node-pre-gyp ERR! Pre-built binaries not found for [email protected] and [email protected] (node-v57 ABI) (falling back to sou
rce compile with node-gyp)
C:\projects\audio-speaker\node_modules\audio-mpg123>if not defined npm_config_node_gyp (node "C:\Users\dmitry\AppData\Ro
aming\npm\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" clean )  else (node "" clean )
C:\projects\audio-speaker\node_modules\audio-mpg123>if not defined npm_config_node_gyp (node "C:\Users\dmitry\AppData\Ro
aming\npm\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" configure --fallback-to-build
--module=C:\projects\audio-speaker\node_modules\audio-mpg123\lib\audio_mpg123.node --module_name=audio_mpg123 --module_p
ath=C:\projects\audio-speaker\node_modules\audio-mpg123\lib --python=C:Python27 --msvs_version=2015 )  else (node "" con
figure --fallback-to-build --module=C:\projects\audio-speaker\node_modules\audio-mpg123\lib\audio_mpg123.node --module_n
ame=audio_mpg123 --module_path=C:\projects\audio-speaker\node_modules\audio-mpg123\lib --python=C:Python27 --msvs_versio
n=2015 )
C:\projects\audio-speaker\node_modules\audio-mpg123>if not defined npm_config_node_gyp (node "C:\Users\dmitry\AppData\Ro
aming\npm\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" build --fallback-to-build --mo
dule=C:\projects\audio-speaker\node_modules\audio-mpg123\lib\audio_mpg123.node --module_name=audio_mpg123 --module_path=
C:\projects\audio-speaker\node_modules\audio-mpg123\lib )  else (node "" build --fallback-to-build --module=C:\projects\
audio-speaker\node_modules\audio-mpg123\lib\audio_mpg123.node --module_name=audio_mpg123 --module_path=C:\projects\audio
-speaker\node_modules\audio-mpg123\lib )
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
  win32.c
C:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared [C:\projects\audio-speaker\node_modules\audio-mpg123\build\src\modu
le.vcxproj]
  win_delay_load_hook.cc
  module.vcxproj -> C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\\module.lib
  compat.c
  compat_str.c
c:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\compat\compat.c) [C:\pr
ojects\audio-speaker\node_modules\audio-mpg123\build\src\compat.vcxproj]
c:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\compat\compat_str.c) [C
:\projects\audio-speaker\node_modules\audio-mpg123\build\src\compat.vcxproj]
  win_delay_load_hook.cc
  compat.vcxproj -> C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\\compat.lib
  legacy_module.c
  libout123.c
  sfifo.c
  stringlists.c
  wav.c
C:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\libout123\legacy_module
.c) [C:\projects\audio-speaker\node_modules\audio-mpg123\build\src\out123.vcxproj]
C:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\libout123\libout123.c)
[C:\projects\audio-speaker\node_modules\audio-mpg123\build\src\out123.vcxproj]
C:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\libout123\stringlists.c
) [C:\projects\audio-speaker\node_modules\audio-mpg123\build\src\out123.vcxproj]
C:\projects\audio-speaker\node_modules\audio-mpg123\src\mpg123\src\compat\compat.h(105): warning C4091: 'typedef ': ign
ored on left of 'long' when no variable is declared (compiling source file ..\..\src\mpg123\src\libout123\wav.c) [C:\pr
ojects\audio-speaker\node_modules\audio-mpg123\build\src\out123.vcxproj]
..\..\src\mpg123\src\libout123\wav.c(151): warning C4244: '=': conversion from 'long' to 'byte', possible loss of data
[C:\projects\audio-speaker\node_modules\audio-mpg123\build\src\out123.vcxproj]
  win_delay_load_hook.cc
  out123.vcxproj -> C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\\out123.lib
  binding.cpp
  win_delay_load_hook.cc
c:\projects\audio-speaker\node_modules\nan\nan_maybe_43_inl.h(220): warning C4996: 'v8::Array::CloneElementAt': was dec
lared deprecated (compiling source file ..\src\binding.cpp) [C:\projects\audio-speaker\node_modules\audio-mpg123\build\
audio_mpg123.vcxproj]
  c:\users\dmitry\.node-gyp\8.1.2\include\node\v8.h(3342): note: see declaration of 'v8::Array::CloneElementAt' (compil
  ing source file ..\src\binding.cpp)
c:\projects\audio-speaker\node_modules\nan\nan_implementation_12_inl.h(40): warning C4996: 'v8::BooleanObject::New': wa
s declared deprecated (compiling source file ..\src\binding.cpp) [C:\projects\audio-speaker\node_modules\audio-mpg123\b
uild\audio_mpg123.vcxproj]
  c:\users\dmitry\.node-gyp\8.1.2\include\node\v8.h(4532): note: see declaration of 'v8::BooleanObject::New' (compiling
   source file ..\src\binding.cpp)
C:\projects\audio-speaker\node_modules\nan\nan.h(1955): warning C4996: 'v8::Object::SetAccessor': was declared deprecat
ed (compiling source file ..\src\binding.cpp) [C:\projects\audio-speaker\node_modules\audio-mpg123\build\audio_mpg123.v
cxproj]
  c:\users\dmitry\.node-gyp\8.1.2\include\node\v8.h(3026): note: see declaration of 'v8::Object::SetAccessor' (compilin
  g source file ..\src\binding.cpp)
  getopt.c
     Creating library C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\audio_mpg123.lib and object C:\
  projects\audio-speaker\node_modules\audio-mpg123\build\Release\audio_mpg123.exp
  Generating code
  Finished generating code
  audio_mpg123.vcxproj -> C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\\audio_mpg123.node
  Copying C:\projects\audio-speaker\node_modules\audio-mpg123\build\Release\/audio_mpg123.node to ./lib/audio_mpg123.no
  de
          1 file(s) copied.
Second, check test does not pass: Could not find audio_mpg123. Should I enable audio_mpg123 manually or something?
@dfcreative Yes, I need to remove those logs from the binding and make some new prebuilt releases for it, due to the newer node versions we are using. I also would like to add the mac prebuilt bindings to it this time as well. As for your log, it looks like the binding built, so I'm not sure why it could not find it. In the audio_mpg123 module inside the lib folder is there a audio_mpg123.node?
@connorhartley ah, right, because of assert here, commenting that out works fine
@dfcreative Ah, my bad. New to assert. 😄
@connorhartley I refactored tests, so please look. It is single test.js entry for both node/browser versions (all tests should pass regardless of platform, hence we should not separate them).
As for assert - the single purpose of it is validating user arguments and that's it, it should not be used for debugging purposes AFAIK, so best of all is removing them from code and keeping the code tidy.
Also I stumbled upon tests inconsistency. In some reason sine has wrong hearable frequency.
UPD. You can npm run test:browser to see how it sounds in browser and compare.
But the fact that we have sound in node is impressive!
@dfcreative the tests are running concurrently. I'm not sure if this is the intended effect?
@connorhartley you mean in browser?
@dfcreative Tests should be run in order after the other is finished. I don't think tape supports this functionality.
EDIT: Just with node, I haven't tested browser yet. npm run test
@connorhartley no, nice catch, seems that I am doing something wrong in web-audio-write - I invoke callback at the wrong moment (after the first frameBlock actually), instead of completing full buffer output.
@connorhartley tape runs sequentially, single-threaded in node and browser, we should not hear tests going off at the same time.
@dfcreative Yeah you're right just re-read tapes docs. Not sure why this is happening to me...
EDIT: I figured it out. I'll push my changes in a sec.
FWIW, I had removed the print functions from the binding in https://github.com/audiojs/audio-mpg123/commit/0f007288920cabce6902a9a8e9451aa230a494c5. We just need a new release on that like @connorhartley mentioned... But managing the prebuilds is a bit of a hassle right now IMO. We need a more clear processes for releases (e.g., open an issue, ping several people who can contribute a builds for different platforms, etc.). Maybe we could just make a release without prebuilds if need be. But even that might require tweaking.
@connorhartley ok, just published [email protected], it runs tests sequentially, and invokes callback only when the planned data is consumed.
@dfcreative thank you for that, I've implemented it in my latest commit.
@connorhartley I see your comments, sorry for the hold on, I am working on pcm-convert module to introduce here, because pcm-util is awful and needs scrapping asap. Also I will do some clean up and resolve conflicts. Meanwhile it would be awesome to resolve mpg123 errors issue, since it should not affect the code here.
I did some upgrades to the index.js, and updated readme and dependencies while I was at it.
I think we should add a test for doing speaker(data, ...) with a buffer?
I think resolving mpg123 errors could come in the NAPI branch? @jamen @dfcreative
@jamen @connorhartley hey I think I am stuck with the thing - I try to output float32 samples to audio-mpg123 - and I get silence. Things went pearshape after introducing pcm-convert, although by itself module works exactly same way as pcm-util/convert, but redesigned to simpler and faster API.
Some disclaimer. There is dtype package, where we can indicate data types way easier as just a string eg. uint8, and that became a standard practice in regl and ndarray. So I think that we could be using a format property here to indicate format as such dtype string. But turns out that messes up output. Although the sine wave I had before the introducing pcm-convert was no better.
I will try to dig deeper what's the problem.
UPD. Figured out that in some reason we have to change endianness from 'le' to 'be' before feeding the data.
@connorhartley @jamen ok, hear proper sound now :tada:
Spotted some bugs though.
- [ ] if we set formatto anything butint16, we get noises
- [x] if we set channels: 2for the generator/speaker, we get high-pitched sine.
@dfcreative I tried to run the tests.  Could u publish [email protected], and then add it in here because it is missing, I was also missing buffer-to-arraybuffer.
Otherwise it is playing good for me too. :smile:
@jamen ok I would make sure there is no bugs outputting any type of any format before publishing (if it is possible). Gonna work tomorrow a bit on that.
@jamen @connorhartley we are almost ready to merge. A couple of things to solve.
- [ ] Now seems like audio-mpg123is incapable of rendering anything butint16format, because changing it to any other breaks normal sound output. Should we convert any data type internally toint16and forget about other formats? Same thing we do with existing audio-speaker impl. Although that would be nice to makeaudio-mpg123produce sound from other formats.
- [ ] If we are going to use internal conversion to int16, we may want to simplify options. Specifically, we don't have to defineformatat all, since we are feeding only audio-buffers.
- [ ] There is a bug with chunkSize: right now it isNaN, and if we try to make it a number, we get really bad sound. Help needed.
- [ ] Not sure I completely understand the meaning of autoFlush- can we elaborate that in readme?
- I don't mind audio-speakeronly accepting AudioBuffers at the moment. But I do agree, it would be nice if the backend could accept different buffer/arraybuffer formats for not only its purposes here, so I would say definitely support that as we continue to make the backend more user-friendly.
- That usage is actually pretty appealing to me. I took a look at the readme and it looks quite a bit more minimal now. Thanks to the 2.0 signature, dtype, and then now finding the format isn't working.
- I'm personally not a fan of autoFlushbeing in the options. I think it should just be implied by this being the official speaker package, because it provides a good default behavior IMO... Whereas inaudio-mpg123, you can flush/open/close/etc. all manually yourself for a more raw, non-streamy way to interact with the speaker. I talked with @connorhartley though and he thought it would be good to at least keep it in, so maybe he has some ideas. :smile:
@jamen no, I am afraid I don't understand not the reason for it to be, but the meaning of that. What does it do, in short? What is the difference between flush and write? (looking with the eyes of user)
I hope it is not leaking API?
@dfcreative It calls out123_drain on the binding after doing out123_play, which will properly drain between writes.  This might have more adverse affects on larger buffers compared to streamed ones (I could be wrong).  But, try commenting all the occurences of mpg123.flush out, and run the tests a few times, see if the playback is buggier than before. It was for me: the noise barely played, lena snippet is shorter than with, and the sine was garbled at the beginning.