Node version? about audio-loader HOT 27 CLOSED

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: [../deps/mpg123/src/output/coreaudio.c:81] warning: Didn't have any audio data in callback (buffer underflow). Maybe you want to take a look.

Regards,
Dani

from audio-loader.

It looks good to me. I'll try to make it work on nodejs soon, so expect
some questions ;-)

El El sáb, 10 sept 2016 a las 22:03, Dima Yv [email protected]
escribió:

I’ve added outline of how it may look here
https://github.com/dfcreative/audio-load/blob/master/index.js.

The points of difference:

• For node we use request and for browser xhr (or fetch polyfill
  if there is such).
• In node we don’t have audio context, and it should be made an
  optional argument. The common practice is passing (maybe) options with
  context.
• For decoding we use audio-decode
  https://github.com/audiojs/audio-decode, in browser it automatically
  uses WebAudioAPI.

Feel free to take part in discussions
https://github.com/audiojs/contributing, share ideas etc. Feel at home)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAdIUTTIgj_2lnyegknKieuZQmshKIpks5qow0KgaJpZM4J5SCH
.

from audio-loader.

Hi @dfcreative

Here's the report of my progress (commit: c14f61b)

• The first thing is that, since make it work it on node means load a lot of extra modules (and module file size in browser matters) I've decided to add audiojs support in a separate file. So instead of require('audio-load') you must write require('audio-load/node'). But I'm open to suggestions.
• I've created a examples/node.js file that runs a simple example (node examples/node.js)
• It works when loading files from filesystem, but it fails when loading from http. Thats because audio-type module can't detect the audio file type. (The error is: [Error: Cannot detect audio format of buffer]). Maybe you can provide some light here
• After playing the sample, it waits more than a second to finish. Thats something you should probably need to polish in audio-play module (and probably related to the small length of the samples I'm using)
• Having an audio environment in node is great!!

Please tell me if you have any clue about the audio file type problem.

Regards,
Dani

from audio-loader.

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,
Dani

from audio-loader.

I’ve added outline of how it may look here.

The points of difference:

• In node we use request and in browser xhr, or polyfill fetch if possible.
• In node we don’t have audio context, and it should be made an optional argument. The common practice is passing (maybe) options with context.
• For decoding we use audio-decode, in browser it automatically uses WebAudioAPI.

Feel free to take part in discussions, share ideas etc. Feel at home)

from audio-loader.

Hi @dfcreative,

I've been playing a little bit with your libraries and I think it will be easy to adapt audio-loader to nodejs environment. Unfortunately, I've made a little test of loading and playing audio and I couldn't hear anything (I don't know if it's a problem on my side or not). The code I used to test was:

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):

2016-09-12 03:12:41.675 node[8012:401831] 03:12:41.675 WARNING:  140: This application, or a library it uses, is using the deprecated Carbon Component Manager for hosting Audio Units. Support for this will be removed in a future release. Also, this makes the host incompatible with version 3 audio units. Please transition to the API's in AudioComponent.h.
Loaded
Ended {}
Ended { '0': true }

I've installed your audio-play library with: npm i --save github:audiojs/audio-play

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 👍

from audio-loader.

That warning looks like a problem of node-speaker, we are going to transit to our own version of it.
You can actually check if it works in browser by budo test.js (budo instead of node). If there is sound - that means it will also work in node. You can install budo globally from npm.

from audio-loader.

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.

from audio-loader.

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.
Try larger wav file and that should work.

from audio-loader.

Actually I resolved this, you can just update audiojs/audio-play and it should work.

from audio-loader.

Great! It works... 👍 I'll work on an adapted version of audio-loader and then we discuss the details.

(Edit:typo)

from audio-loader.

Nice!

1. For that purpose there is browser field in package.json, so that the main package entry is node version, and usually /browser.js is for browser. Also js file named node.js messes up with node itself when I try to launch it in windows. Mb it is better to rename browser version to browser.js and keep index.js for node, and add "browser": "browser.js" to package.json.
2. Pass encoding: null instead of encoding: undefined to sendRequest to receive binary data as a response, should work. That was my mistake in audio-load.
3. The 2s of silence is bad workaround in audio-play for now, otherwise that ignores output for short sounds. It should be fixed in some future, not sure how bad it is.

Thanks, looking forward to using it in audio!

from audio-loader.

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 guess tests also should be able to be run in node, right?

from audio-loader.

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,
Dani

from audio-loader.

Not sure why Buffer.from(buffer) does not work for you, there is no difference between Buffer.from(array) and new Buffer(array) afaik, just tested that.

The function signature could be changed at least to avoid load(null, url), because it looks a bit confusing in code in this case. My intuition is load(what, how), or generally change(what, how), but I see that you use context as a first argument across many packages, so that might be a trouble. So possibly just check if first argument is not context, so that function can be both

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.

from audio-loader.

Here you have more information about the error: https://www.npmjs.com/package/typedarray-to-buffer
Maybe it's because my node it's too old (4.2). What versions of node are you planning to support?

I agree is weird to have an null as first parameter.

from audio-loader.

Ok, I’ve updated audio-decode, now it does not copy typed array.

from audio-loader.

Hi @dfcreative

I've made lot of changes, and is working... more or less. I'm having troubles in the following areas:

• Even when the example is working on node, I can't make the test passing on that environment (currently are skipped)
• When building the distribution file with browserify, it includes some (unnecessary) extra library (see dist/audio-loader.js)
• The node example throws lot of warnings and sometimes it get stalled

I finally remove the AudioContext from the function signature ;-)

I think when that issues are solved, the work is done.

Regards,
Dani

from audio-loader.

Update: some node examples throws errors on my computer (OSX, Node 4):

/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/mp3/src/header.js:135
        throw new AV.UnderflowError(); // LOSTSYNC
        ^
Error
    at new UnderflowError (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/av/src/core/underflow.js:13:20)
    at MP3FrameHeader.decode (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/mp3/src/header.js:135:15)
    at Function.MP3FrameHeader.decode (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/mp3/src/header.js:237:16)
    at Class.readChunk (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/mp3/src/demuxer.js:139:41)
    at BufferSource.<anonymous> (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/av/src/demuxer.js:49:19)
    at BufferSource.EventEmitter.emit (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/av/src/core/events.js:64:12)
    at BufferSource.loop (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/av/src/sources/buffer.js:49:21)
    at Immediate._onImmediate (/Users/Dani/Code/Js16/audio-loader/node_modules/audio-decode/node_modules/av/src/sources/buffer.js:4:59)
    at processImmediate [as _immediateCallback] (timers.js:368:17)

from audio-loader.

Hi Dani!

That is awesome! I like the restructuring!
I’ve added some comments to your last commit.

1. In node use is-audio-buffer package instead of buffer instanceof Buffer. The last never works btw.
2. It is better to avoid bundling in case if audio-loader. But I also noticed that you use Buffer object in browser files — try removing any mention of it, because by itself Buffer module is ~40kb of code. That should help with the size. Also you can see the list of included packages as: browserify . --list
3. The errors of decoding are related to the short duration of samples. These are errors of decoders: aurora.js and wav-decoder. We should address them there. For now try to use larger files if possible.

• I would also recommend adding 'use strict'; to the top of every file and setup eslint, but I can do a PR for that.
• Also here you have base64 decoder, same as in audio-lena, we can think of merging that part of code one day.
• Think of possible option {context: myAudioContext} (in case if user has own audio context), now it is not mentioned in docs.

All in all, great, looking forward to release!
Btw new API is lovely!

from audio-loader.

Hi @dfcreative,

I'm glad you like it. Thanks a lot for the comments (from the source also). Here some thoughts:

1. Understood. Done.
2. Great, you found the root of the dependency problem. I know that is highly improbable that audio-loader is used without browserify or similar tool, but I like to bundle it mostly to test that I'm doin' it right (like detect unused dependencies, like this one)
3. Ok. I've made a commit where the test works with the the wav... but fails with mp3. Arggh.
4. Yes, I forgot. Done.
5. It sounds good to me.
6. Currently pass the audio context is not supported. Since the only method it needs from audio context is decodeAudioData you can pass the method alone. It's not documented because I'm not 100% sure if it's the right path.

When I try to run the test with an .mp3 file I get the following error:

at new UnderflowError (node_modules/audio-decode/node_modules/av/src/core/underflow.js:13:20)
      at MP3FrameHeader.decode (node_modules/audio-decode/node_modules/mp3/src/header.js:135:15)
      at Function.MP3FrameHeader.decode (node_modules/audio-decode/node_modules/mp3/src/header.js:237:16)
      at Class.readChunk (node_modules/audio-decode/node_modules/mp3/src/demuxer.js:139:41)
      at BufferSource.<anonymous> (node_modules/audio-decode/node_modules/av/src/demuxer.js:49:19)
      at BufferSource.EventEmitter.emit (node_modules/audio-decode/node_modules/av/src/core/events.js:64:12)
      at BufferSource.loop (node_modules/audio-decode/node_modules/av/src/sources/buffer.js:49:21)
      at Immediate._onImmediate (node_modules/audio-decode/node_modules/av/src/sources/buffer.js:4:59)

I think we are really close to a release 👍 Thanks for the idea and the support.

from audio-loader.

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 sampleRate and other context params. Though that is a bit more difficult for the user than just passing newly created context - he has to remember the name context.decodeAudioData.

Anyways it’s up to you to decide, that is awesome we progressed so far.

from audio-loader.

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?

from audio-loader.

We can remove this wrapper (audio-decode does that already) and we’re ready for release I think 🎆

from audio-loader.

Before the release I've think I'll follow you advice and add context as another option 👍

Anything else before publish? 1, 2, 3...

from audio-loader.

Lgtm, there may be issues when I try to integrate into audio, but for now looks good.

from audio-loader.

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.

from audio-loader.