-
Notifications
You must be signed in to change notification settings - Fork 18
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
Node version? #2
Comments
Hi @dfcreative, I'm glad you like the module. I don't know how to make it works on nodejs since I've never try work with audio in that environment, but it sounds appealing! Can you explain a little bit how to do it? I would happy accept some pull request or we can see how to integrate with your library. Thanks for the invitation too! Regards, |
I’ve added outline of how it may look here. The points of difference:
Feel free to take part in discussions, share ideas etc. Feel at home) |
It looks good to me. I'll try to make it work on nodejs soon, so expect El El sáb, 10 sept 2016 a las 22:03, Dima Yv notifications@github.com
|
Hi @dfcreative, I've been playing a little bit with your libraries and I think it will be easy to adapt var fs = require('fs')
var decodeAudio = require('audio-decode')
var play = require('audio-play')
var CLAVE = './example/samples/maeclave.wav'
function load (file) {
return new Promise(function (done, reject) {
fs.readFile(file, function (err, file) {
err ? reject(err) : done(file)
})
})
}
function decode (data) {
return new Promise(function (done, reject) {
decodeAudio(data, function (err, buffer) {
err ? reject(err) : done(buffer)
})
})
}
load(CLAVE).then(decode).then(function (buffer) {
console.log('Loaded')
play(buffer, {
autoplay: true
}, function () {
console.log('Ended', arguments)
})
}).catch(function (err) {
console.log('Error', err)
}) The output in my machine (osx):
I've installed your Any ideas? Anyway, It would be great to use audio in node. Thanks for your effort and libraries! They look really nice, let's do it 👍 |
That warning looks like a problem of node-speaker, we are going to transit to our own version of it. |
I don't know what you mean... What can I check? Not the code I had write, of course, because of all the node calls... Or I am missing something? Is there any way I can play a buffer in node different from the one I've tried? Sorry for the confusion. |
Ah, I see. That is the problem of the node-speaker package we use to output sound. It has large buffering and if your audio buffer is less than ~2s it will just ignore the output. It’s a bug audiojs/audio-speaker#21. |
Actually I resolved this, you can just update |
Great! It works... 👍 I'll work on an adapted version of (Edit:typo) |
Hi @dfcreative Here's the report of my progress (commit: c14f61b)
Please tell me if you have any clue about the audio file type problem. Regards, |
Nice!
Thanks, looking forward to using it in audio! |
Hi, I've made another commit. It seems to work with single audio files, multiple encoded json audio and soundfont files. Some things must be polished (like the point 1 of the previous comment) but the basic structure is done. When running the example in my machine, I get a lot of warnings: Regards, |
Sweet, it works! I have no such warnings, though I use windows platform. Probably for large wav files it will not be a big deal, though it is an issue.. I referenced it in audiojs/audio-speaker#22 |
I won't be able to work more on this today, but I forgot to comment one issue: as you can see in line 8 of node.js file, I had to patch audio-decode. The thing is that, in audio decode it uses Buffer.from(buffer) to convert from ArrayBuffer to Buffer, but this doesn't work if the audio is encoded, because it uses a Uint8Array. Any solution proposal is welcomed ;-) I hope tomorrow I have time so I can fix tests and prepare a new release of the library (I'm still in doubt about the function signature changes, and some other minor issues) Regards, |
Not sure why The function signature could be changed at least to avoid load(context, url);
//and
load(url);
//which is the same as
load(null, url) But honestly I would reconsider forcing user to create context manually even in browser (there are vendor prefixes etc), to allow him just do load(url).then(play); This way user would not care about the way audio is done. |
Here you have more information about the error: https://www.npmjs.com/package/typedarray-to-buffer I agree is weird to have an null as first parameter. |
Ok, I’ve updated audio-decode, now it does not copy typed array. |
Hi @dfcreative I've made lot of changes, and is working... more or less. I'm having troubles in the following areas:
I finally remove the AudioContext from the function signature ;-) I think when that issues are solved, the work is done. Regards, |
Update: some node examples throws errors on my computer (OSX, Node 4):
|
Hi Dani! That is awesome! I like the restructuring!
All in all, great, looking forward to release! |
Hi @dfcreative, I'm glad you like it. Thanks a lot for the comments (from the source also). Here some thoughts:
When I try to run the test with an .mp3 file I get the following error:
I think we are really close to a release 👍 Thanks for the idea and the support. |
Bundling won’t save from unused dependencies, eslint for that purpose is way better, that also detects unused vars etc. You can use very loose config from audiojs. With mp3 I have the same issue, I guess we need to try larger mp3. As for restricting user passing own context there is a problem. Web-Audio-API supports only 6 context instances at the same time (depends on machine, here it is 999 there it is 1). So if user already used all of them, he won’t be able to use audio-loader. Passing decode function may work though, as far as it does not require Anyways it’s up to you to decide, that is awesome we progressed so far. |
Hi, I've added a longer mp3 to test and it works. 👏 Your're right about the AudioContext, so I've updated the README to document about it. I use standard style, and an in-editor standard-eslint plugin. But I'm open to suggestions (and PR) For me the 0.6.0 release is ready. What do you think? |
We can remove this wrapper (audio-decode does that already) and we’re ready for release I think 🎆 |
Before the release I've think I'll follow you advice and add Anything else before publish? 1, 2, 3... |
Lgtm, there may be issues when I try to integrate into audio, but for now looks good. |
Hi @dfcreative. I made a release, so I'll close this issue. Please open a new one with any issue you encounter. Nice to talk with you. |
Hi @danigb!
First off, thanks for the very nice module.
I am thinking to use it in upcoming audiojs/audio version, and audio-loader is almost perfect. One thing though — is it possible to make it work in nodejs?
I see right now you take
ac
as a first argument everywhere, but we use audio-context singleton to make that argument optional. That is suitable in case of nodejs, where having context is not necessary.Appreciate any response.
The text was updated successfully, but these errors were encountered: