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

Own speaker implementation #2

Open
dy opened this issue Feb 10, 2016 · 80 comments
Open

Own speaker implementation #2

dy opened this issue Feb 10, 2016 · 80 comments
Assignees

Comments

@dy
Copy link
Member

dy commented Feb 10, 2016

npm install audio-speaker gives

C:\Users\dmitry\Dropbox\Projects\audio-through\node_modules\nan\nan.h(41): fatal error C1189: #error :  This version of
 node/NAN/v8 requires a C++11 compiler [C:\Users\dmitry\Dropbox\Projects\audio-through\node_modules\speaker\build\bindi
ng.vcxproj]

This happens only when local version of node-gyp is used. Probably some config needed or something. Because global node-gyp rebuilds speaker just fine.

@dy
Copy link
Member Author

dy commented Feb 10, 2016

Or maybe that is a perfect reason to study prebuild. Because it’s annoying to wait for TooTallNate to merge speaker as well as the whole situation with absence of sound in node is a sort of crap - it’s 2016, we’ve sent satellites to Mars.

@dy
Copy link
Member Author

dy commented May 9, 2016

speaker also really needs .npmignore.
Guess we need own implementation.

@dy dy changed the title Installation issue Own speaker implementation Aug 26, 2016
@vectrixdevelops
Copy link

I can work on this if you'd like? @dfcreative

@dy
Copy link
Member Author

dy commented Aug 27, 2016

Wow, that would be awesome

@vectrixdevelops
Copy link

vectrixdevelops commented Aug 27, 2016

I'm just creating a mirror of the mpg123 svn repo to git, so we can easily keep the library up to date, give credit to those who made it and let people modify it through git. Then we can add it here as a submodule and make the binding.

I'm not the best with C++ and C, but we'll put it on a separate branch in case someone else can contribute to it if I can't make it.

@vectrixdevelops vectrixdevelops self-assigned this Aug 27, 2016
@dy
Copy link
Member Author

dy commented Aug 27, 2016

Yeah, that is good to have mpg123 updated automatically. The point as I get it is not C/C++ but node bindings, that takes good knowledge of v8.
Speaking practice, running build on npm install is always a headache for npm users. For example, for now to use node-speaker for windows users we need installing microsoft SDKs, which are 7Gb of extra space, just for tiny module.
I tried to research node prebuild to avoid that, but that was too difficult for me..

@vectrixdevelops
Copy link

vectrixdevelops commented Aug 27, 2016

Alright, I will look into prebuild and yeah the bindings will be difficult, but it's worth giving it a shot. For windows building maybe this is a temporary fix https://www.npmjs.com/package/windows-build-tools although I have found it doesn't set the python path very well, although they are working on a fix I believe. It also requires cmd or powershell to run in admin.

@vectrixdevelops
Copy link

I have got a mirror setup now. I will start working on the binding. https://github.com/connorhartley/mpg123

@vectrixdevelops
Copy link

Just some information on what I've done. Basically I have forked mpg123-mirror from my profile to audiojs to make modifications specifically for audio-speaker. Currently binding-implementation compiles on Windows fine. I am going to focus on finishing the binding for Windows then we will worry about porting this to other operating systems and arch types. I will need help with testing this on the other systems afterwards.

@dy
Copy link
Member Author

dy commented Aug 29, 2016

Yeah, I am also on windows, cc @jamen

@jamen
Copy link
Contributor

jamen commented Aug 29, 2016

Linux Mint here.

@vectrixdevelops
Copy link

The C++ binding stuff is done, now all we'll need is the JS implementation for this. I'm open to suggestions on implementation. @dfcreative @jamen @ahdinosaur @mmckegg

@dy
Copy link
Member Author

dy commented Sep 3, 2016

@connorhartley
The branch seems to be an orphan, it has no master’s history. For pull-request we will have to create a new branch I guess.

Also now it’s a bit tricky to figure out how to install mpg123. That would be nice to add that to readme.

Also that is must-have package.json with build script.

Also I do

node-gyp clean;
node-gyp configure;
node-gyp build;

...

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(55,5): error MSB8020: The build tools for Visual Studio 2010 (Platform Toolset = 'v100') cannot be found. To build using the v100 build tools, please install Visual Studio 2010 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\module.vcxproj]

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(55,5): error MSB8020: The build tools for Visual Studio 2010 (Platform Toolset = 'v100') cannot be found. To build using the v100 build tools, please install Visual Studio 2010 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\compat.vcxproj]

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(55,5): error MSB8020: The build tools for Visual Studio 2010 (Platform Toolset = 'v100') cannot be found. To build using the v100 build tools, please install Visual Studio 2010 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\mpg123.vcxproj]

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(55,5): error MSB8020: The build tools for Visual Studio 2010 (Platform Toolset = 'v100') cannot be found. To build using the v100 build tools, please install Visual Studio 2010 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\out123.vcxproj]

Not sure what’s up.

@vectrixdevelops
Copy link

vectrixdevelops commented Sep 3, 2016

@dfcreative
Yeah I will rebase before I merge to master.

I will add the readme very soon, along with instructions for configuring mpg123 a long with build instructions.

What version of Visual Studio are you using?

EDIT

The error can be caused by multiple reasons, try the following.

npm install --msvs_version=2015

Alternatively to save time in the future for building, you can set an environment variable.

Variable: GYP_MSVS_VERSION Value: 2015

If that doesn't work, I would take a look at the other options in this SO answer I found.
http://stackoverflow.com/questions/32556295/npm-install-error-the-build-tools-for-v120-platform-toolset-v120-cannot

If that doesn't work, let me know. 😄

@dy
Copy link
Member Author

dy commented Sep 3, 2016

With vs2015 flag:

C:\Users\dmitry\Dropbox\projects\audio-\speaker\src\mpg123\src\libout123\out123_int.h(12): fatal error C1083: Cannot op
en include file: 'config.h': No such file or directory [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\modul
e.vcxproj]
  compat.c
  compat_str.c
..\..\src\mpg123\src\compat\compat.c(12): fatal error C1083: Cannot open include file: 'config.h': No such file or dire
ctory [C:\Users\dmitry\Dropbox\projects\audio-\speaker\build\src\compat.vcxproj]
c:\users\dmitry\dropbox\projects\audio-\speaker\src\mpg123\src\compat\compat.h(17): fatal error C1083: Cannot open incl
ude file: 'config.h': No such file or directory (compiling source file ..\..\src\mpg123\src\compat\compat_str.c) [C:\Us
ers\dmitry\Dropbox\projects\audio-\speaker\build\src\compat.vcxproj]
  parse.c
...

Basically config.h is not found, feels like mpg123 needs to be configured or something.

@vectrixdevelops
Copy link

vectrixdevelops commented Sep 4, 2016

@dfcreative
Could you paste the whole error (specifically gyps info) as I can't seem to reproduce the error?

EDIT

The latest commit may fix your problem, as the target arch default was ia32 (maybe your arch type is also ia32) and I hadn't made a configuration for it. If you are running x64 use --target-arch=x64 when building and configuring. Note: I will put this information in the readme after.

@dy
Copy link
Member Author

dy commented Sep 5, 2016

@connorhartley btw considering the latest changes in @audiojs, for own implementation it would be enough to have write(buffer, cb) function, not necessary to implement stream wrapper

@vectrixdevelops
Copy link

@dfcreative did that error get fixed?

@dy
Copy link
Member Author

dy commented Sep 9, 2016

Naah, still get the same error - config.h not found. I just copied the code of audiojs/mpg123 into src/mpg123 folder, I guess it has to be configured or something... When I tried to do the same thing before with updated mpg123 I got the same error and just gave up

@vectrixdevelops
Copy link

@dfcreative have you tried setting the --target-arch flag in the build and configure command?

e.g
node-gyp configure --target-arch=x64
node-gyp build --target-arch=x64

If the problem persists please let me know. 😄

@Elemino
Copy link

Elemino commented Mar 4, 2019

Thanks for the clarification. I will update with my findings shortly.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Mar 5, 2019

@dy I would like to get up to speed with this and I thought if somebody could share their build script for npm run build, that would be great. As I understand it the config.h error happens after trying to reconfigure node-gyp. I would like to understand better if anybody else other than @dy has been able to reproduce the config.h. I would like to really understand deeply how each library component works and why you guys need/want it. Not in terms of audio capabilities level (as of yet) but just on a 'build' level for now. Another question I have potentially, does napi allow to switch versions without recompiling/rebuilding just to avoid the nan problem as seen on this issue? Is using napi with mpg123 instead of nan viable? Because if there's a missing win32 identifier problem (as the error states) maybe there's a wrong abi (configured for 64bit) and some value is not being read. I might be talking nonsense but I just want to gauge the surface and learn more. What are your thoughts? One thing is for sure, implementing a new cubeb cross-platform with napi will be somewhat groundbreaking. I think of all those native speed low latency audio streaming possibilities. Or maybe the new features you're after for the speaker output are different. First thing is first though. Once I cover all possibilities that may cause the error as it pertains to the gyp mgpg123 binding and the configure command, I'll move onto other things.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Mar 5, 2019

I guess there's no abi in nan.

@dy
Copy link
Member Author

dy commented Mar 6, 2019

@Elemino I didn't work much with low-level API, I was mostly concerned with consistent modules interface and optimal js code. That question is better addressed to @jamen or @connorhartley, the guys spend more time on that (thanks for that!)

@jamen
Copy link
Contributor

jamen commented Mar 6, 2019

As I understand it the config.h error happens after trying to reconfigure node-gyp. I would like to understand better if anybody else other than @dy has been able to reproduce the config.h.

config.h files make Mpg123 work for different systems. They're from audio-mpg123's source, copied from node-speaker's source when we forked it. The comments seem to explain how it works, but I haven't read through the files personally.

I haven't reproduced this error. It might not even be relevant anymore? There is several issues and incomplete parts of the bindings, though.

I would like to really understand deeply how each library component works and why you guys need/want it. Not in terms of audio capabilities level (as of yet) but just on a 'build' level for now.

I'll talk about Mpg123's setup, Cubeb's setup, then our setups.

Visit Mpg123's website for a lot of information. Mpg123 is hosted on SVN, so its mangled into git projects. audiojs/mpg123-mirror is @connorhartley's port to git and node-speaker includes mpg123 as a directory. See also API documentation for libmpg123 and libout123.

Mpg123 uses automake for its build, but it has to be reworked in node-gyp for our binding. Luckily node-speaker already did this (found in our fork here). If you feel motivated to rework the build of the binding, I'm open to ideas. For example, in Cubeb I've reached a simpler solution without node-gyp.

Clone Cubeb's repository and build its docs for information (I don't know any online sources unfortunately). Cubeb uses CMake for its build, so I've used cmake-js in the binding, which makes the entire thing much simpler.

Going forward with either one, it should use prebuild eventually, where instead of compiling source code when its installed, you install the compiled binary for your system.

Not sure what else to add here.

Another question I have potentially, does napi allow to switch versions without recompiling/rebuilding just to avoid the nan problem as seen on this issue?

You should recompile when switching Node versions. I've gotten errors about mismatched ABI versions several times. Not sure if it absolutely necessary, though.

The original issue could be related this. But, it would've been a local fix, and there is other issues with the bindings.

Is using napi with mpg123 instead of nan viable? Because if there's a missing win32 identifier problem (as the error states) maybe there's a wrong abi (configured for 64bit) and some value is not being read. I might be talking nonsense but I just want to gauge the surface and learn more. What are your thoughts?

It is! We have an incomplete branch for this, which you are welcome to continue, or start your own.

I would rank all the options we have like this:

  1. Cubeb + N-API
  2. Mpg123 + N-API
  3. Cubeb + NAN
  4. Mpg123 + NAN

Any of these are acceptable if its more stable/complete than what we have now. Just points to be made about which are better.

One thing is for sure, implementing a new cubeb cross-platform with napi will be somewhat groundbreaking. I think of all those native speed low latency audio streaming possibilities. Or maybe the new features you're after for the speaker output are different. First thing is first though. Once I cover all possibilities that may cause the error as it pertains to the gyp mgpg123 binding and the configure command, I'll move onto other things.

Definitely!

I want to stress the point: Mpg123 and Cubeb are alternatives to doing the same thing. Cubeb arguably does it better. Mpg123 is already began and undergone testing. But we need to choose one of them to use in audio-speaker.

I don't have complete insight in how Web Audio API, Mpg123, and Cubeb stack up against each other with their performance and features, but in my opinion, Web Audio API looks pretty robust as it is, we should use it where we can, and Cubeb where we can't.

First thing is first though. Once I cover all possibilities that may cause the error as it pertains to the gyp mgpg123 binding and the configure command, I'll move onto other things.

If you want to champion both of bindings, keep in mind we can only use one for audio-speaker.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Mar 9, 2019

This clarifies so much @jamen thank you. Will touch base again shortly.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

10 similar comments
@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

gitcoinbot commented Mar 25, 2019

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Mar 31, 2019

Okay, I've been absent for a while. I will submit WIP PR Monday evening to start documenting my progress. I hope this will add some value. Thanks.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Apr 3, 2019

Okay, I'm almost ready to submit the first WIP PR. I apologize it took so long. I had to learn C++ from scratch and had some family issues along the way. Quick question: Should I start with a new branch or submit a fork? Thank you all for your patience.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@jamen
Copy link
Contributor

jamen commented Apr 4, 2019

@Elemino No rush its all good. You can submit a PR from a fork.

@gitcoinbot
Copy link

@Elemino Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@Elemino
Copy link

Elemino commented Sep 2, 2019

Hello there, it's been a while ever since I've posted anything on here. I have some time to put into this project, so I thought I'd let you know. I'll see if I can submit something soon. Please let me know if there are any changes in the ecosystem that you would like me to know about. p.s: If this bounty could be revived, that would be great! It got expired at some point but gitcoin seemingly allows to submit work? Thanks a lot and I hope you all are doing good.

@Elemino
Copy link

Elemino commented Sep 3, 2019

denoland/deno#2825 Does this qualify as a solution?

@dy
Copy link
Member Author

dy commented Sep 3, 2019

Hm? deno supports sound?

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

No branches or pull requests

7 participants