edimuj / cordova-plugin-audioinput

This iOS/Android Cordova/PhoneGap plugin enables audio capture from the device microphone, by in near real-time forwarding audio to the web layer of your application. A typical usage scenario for this plugin would be to use the captured audio as source for a web audio node chain, where it then can be analyzed, manipulated and/or played.
https://github.com/edimuj/app-audioinput-demo
MIT License
161 stars 88 forks source link

raw audio mode sends data as a string? #41

Closed mreinstein closed 7 years ago

mreinstein commented 7 years ago

@edimuj I noticed these lines in the code:

var audioData = JSON.parse(audioInputData.data);

(from https://github.com/edimuj/cordova-plugin-audioinput/blob/master/www/audioInputCapture.js#L215)

NSString *str = [mutableArray componentsJoinedByString:@","];
NSString *dataStr = [NSString stringWithFormat:@"[%@]", str];
NSDictionary* audioData = [NSDictionary dictionaryWithObject:[NSString stringWithString:dataStr] forKey:@"data"];

(from https://github.com/edimuj/cordova-plugin-audioinput/blob/master/src/ios/CDVAudioInputCapture.m#L93-L95)

It seems that the obj-c code is converting an array of (short ints?) to a string, and then the javascript code is parsing that into a plain javascript array.

I'm wondering if there's an opportunity here to make this more efficient: Instead of casting to/from a string, can we somehow send the array directly to javascript?

mreinstein commented 7 years ago

Looking at https://cordova.apache.org/docs/en//latest/guide/platforms/ios/plugin.html#ios-cdvpluginresult-message-types

It seems like what we really want is to send the audio data to javascript as an ArrayBuffer. I think this could massively improve plugin performance. I think doing this would solve #4 too.

mreinstein commented 7 years ago

I've played around with this some more. In CDVAudioCapture.m:

- (void)didReceiveAudioData:(short*)data dataLength:(int)length
{
    [self.commandDelegate runInBackground:^{
        @try {
            if(length > 0) {
                NSData* audioData = [NSData dataWithBytes:data length:length];
                if (self.callbackId) {
                    CDVPluginResult* result = [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArrayBuffer:audioData];
                    [result setKeepCallbackAsBool:YES];
                    [self.commandDelegate sendPluginResult:result callbackId:self.callbackId];
                }
            }
        }
        @catch (NSException *exception) {
            if (self.callbackId) {
                CDVPluginResult* result = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR
                messageAsString:@"Exception in didReceiveAudioData"];
                [result setKeepCallbackAsBool:YES];
                [self.commandDelegate sendPluginResult:result callbackId:self.callbackId];
            }
        }
    }];
}

In AudioInputCapture.js:

/**
 * Callback for audio input
 *
 * @param {Object} audioData   ArrayBuffer containing PCM audio data
 */
audioinput._audioInputEvent = function (audioData) {
    try {
        if (audioData && audioData.byteLength > 0) {
            // no longer needed?
            //audioData = audioinput._normalizeAudio(audioData);

            if (audioinput._cfg.streamToWebAudio && audioinput._capturing) {
                audioinput._enqueueAudioData(audioData);
            }
            else {
                cordova.fireWindowEvent("audioinput", { data: audioData });
            }
        }
        else if (audioInputData && audioInputData.error) {
            audioinput._audioInputErrorEvent(audioInputData.error);
        }
    }
    catch (ex) {
        audioinput._audioInputErrorEvent("audioinput._audioInputEvent ex: " + ex);
    }
};

This almost works; it compiles, and it produces audio but it doesn't sound right. Any thoughts on what I'm doing wrong?

edimuj commented 7 years ago

Thanks for looking into this @mreinstein .

I don't quite remember why I chose to send the data as a string, but it was probably due some problems like the above combined with limited time. It looks like you're onto the solution. I looked into this a little bit more now, thanks to you, and noticed this in the Cordova docs for iOS plugin development:

"messageAsArrayBuffer expects NSData and converts to an ArrayBuffer in the JavaScript callback. Likewise, any ArrayBuffer the JavaScript sends to a plugin are converted to NSData"

The disorted audio, sounds like it could be some kind of rounding or datatype conversion error. It is pretty late here, but I'm happy to experiment a little more with the fix tomorrow. Would be really really nice to fix this bottleneck :)

mreinstein commented 7 years ago

The disorted audio, sounds like it could be some kind of rounding or datatype conversion error.

I'm wondering about that too. In my above code, I've disabled the call to audioinput._normalizeAudio(audioData) because It's unclear why it divides every sample by 32767, or why the last value of the array might be a NaN value.

I'm under the impression that the data argument in (void)didReceiveAudioData:(short*)data dataLength:(int)length is in signed int16 format. Is the goal to convert that from signed 16 bit +/- 32767 to +/- 1.0 floating point? If so, why the conversion to float? Is this for the sake of compatibility with webaudio?

In my application, I hope to take the raw audio and pass it through a websocket stream, so other services can consume it as 16 bit, 16khz, mono, little endian PCM.

Would be really really nice to fix this bottleneck :)

Agreed! The current cordova-plugin-audioinput is working pretty nicely. My only complaint is that there seems to be multiple levels of conversion going on, so anything we can do to make it more efficient is great! (less conversion, working with typed arrays, etc.) Happy to help in any way that I can.

Great work on this plugin by the way, a real life saver!

edimuj commented 7 years ago

Thanks! Ok, I've had a chance to look into this now a little bit more.

If I have understood the facts correctly, it seems that ArrayBuffers passed between the native and web layers in Cordova plugins always is serialized using base64 encoded strings. So improving performance by using ArrayBuffers instead of the current way which relies on String, probably has no positive impact on performance. I also looked into the native code for Android, and the same applies there as well.

Reference: https://github.com/apache/cordova-ios/blob/master/CordovaLib/Classes/Public/CDVPluginResult.m Look at the CDVPluginResult.massageMessage method.

Regarding the conversion to float: Yes, it is for compatibility with web audio, so disabling it breaks that particular feature. If you just need the raw data, that should be possible by just setting the normalize parameter in the capture configuration to false instead of the default which is true. That should hopefully solve your specific case and should improve performance since it then skips the conversion to float altogether.

I haven't been able to pinpoint why the last value of the array sometimes is NaN. It is passed that way from the AudioQueueInput and it only seems to happen on iOS.

mreinstein commented 7 years ago

ok well I guess there's not much to be done here. I suspect the code could be simplified a bit but probably not worth it if there's no big perf game. Disappointing that base64 encoding is the serialization format. :( Thanks for looking into this though!

edimuj commented 7 years ago

No problem, thanks for your interest in improving the plugin. Yes, I'm disappointed as well about the base64 serialization. Though I believe the mobile webviews don't have that type of functionality out-of-box; but passing ArrayBuffers or even direct streaming between the native and weblayers would indeed be a game changer, if that ever gets implemented.

mreinstein commented 7 years ago

@edimuj I just found out today that upcoming safari 11 plans to support media streaming and webrtc. Since this is already present in Android/Chrome this might deprecate the need for this plugin:

https://developer.apple.com/library/content/releasenotes/General/WhatsNewInSafari/Safari_11_0/Safari_11_0.html

exciting stuff!

edimuj commented 7 years ago

Yes, I read it too. It looks like they will roll it out as Google did, when they introduced webrtc in chrome: first for desktop safari (with a beta period), then for mobile Safari and last for the webview, so it will probably take a while. It is good news though! 👍