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

Add Android audio input support to JSynAndroidAudioDeviceManager #20

Closed
Calsign opened this issue Dec 17, 2018 · 14 comments
Closed

Add Android audio input support to JSynAndroidAudioDeviceManager #20

Calsign opened this issue Dec 17, 2018 · 14 comments
Labels
Android bug Something isn't working help wanted Extra attention is needed

Comments

@Calsign
Copy link
Contributor

Calsign commented Dec 17, 2018

Hey, this library looks like great work and I'm super excited to see that it is compatible with Android!

I have tried all of the examples in Android mode, and all of them work great with the exception of the AudioInput example. When running the sketch on my device, I get the following stack trace:

java.lang.RuntimeException: Audio Input not configured in start() method.
	at com.jsyn.engine.SynthesisEngine.getInputBuffer(Unknown Source:13)
	at com.jsyn.unitgen.ChannelIn.generate(Unknown Source:11)
	at com.jsyn.unitgen.UnitGenerator.pullData(Unknown Source:44)
	at com.jsyn.ports.PortBlockPart.pullData(Unknown Source:19)
	at com.jsyn.ports.UnitInputPort.pullData(Unknown Source:11)
	at com.jsyn.unitgen.UnitGenerator.pullData(Unknown Source:40)
	at com.jsyn.ports.PortBlockPart.pullData(Unknown Source:19)
	at com.jsyn.ports.UnitInputPort.pullData(Unknown Source:11)
	at com.jsyn.unitgen.UnitGenerator.pullData(Unknown Source:40)
	at com.jsyn.ports.PortBlockPart.pullData(Unknown Source:19)
	at com.jsyn.ports.UnitInputPort.pullData(Unknown Source:11)
	at com.jsyn.unitgen.UnitGenerator.pullData(Unknown Source:40)
	at com.jsyn.ports.PortBlockPart.pullData(Unknown Source:19)
	at com.jsyn.ports.UnitInputPort.pullData(Unknown Source:11)
	at com.jsyn.unitgen.UnitGenerator.pullData(Unknown Source:40)
	at com.jsyn.engine.SynthesisEngine.synthesizeBuffer(Unknown Source:32)
	at com.jsyn.engine.SynthesisEngine.generateNextBuffer(Unknown Source:27)
	at com.jsyn.engine.SynthesisEngine$EngineThread.run(Unknown Source:194)

I was wondering if this was an Android permissions issue, so I modified the example to request the RECORD_AUDIO permission, but I still got the same error message.

I don't know how much you intend to support Android mode, but the degree to which things do work is already quite impressive and a significant improvement over the previous lack of sound solutions for Android. In other words, it would be nice for audio input to work on Android, but if Java mode is more of a priority then that is understandable.

@kevinstadler I am willing to look into this issue myself, but I was wondering if you had any insight into what the issue is because you obviously have more experience with this project.

@kevinstadler kevinstadler added bug Something isn't working help wanted Extra attention is needed labels Jan 24, 2019
@kevinstadler
Copy link
Collaborator

kevinstadler commented Jan 24, 2019

Sorry for the late response and thanks so much for testing out Android mode! I'm not sure what exactly the problem is but can give you some pointers: the only Android-specific class is
processing.sound.JSynAndroidAudioDeviceManager, so if there's an easy fix to the problem it will probably be in there. I've gut a hunch that the problem is simply that the Android Media input-AudioTrack is not properly initialised in the AndroidAudioInputStream sub-class' start() method:

private class AndroidAudioInputStream extends AndroidAudioStream implements AudioDeviceInputStream {
public AndroidAudioInputStream(int deviceID, int frameRate, int samplesPerFrame) {
super(deviceID, frameRate, samplesPerFrame);
}
public void start() {
}
public double read() {
double[] buffer = new double[1];

My experience with Android audio is limited, but something parallel to the start() method of the corresponding AndroidAudioOutputStream sub-class should probably work:
private class AndroidAudioOutputStream extends AndroidAudioStream implements AudioDeviceOutputStream {
public AndroidAudioOutputStream(int deviceID, int frameRate, int samplesPerFrame) {
super(deviceID, frameRate, samplesPerFrame);
}
public void start() {
this.minBufferSize = AudioTrack.getMinBufferSize(this.frameRate, AudioFormat.CHANNEL_OUT_STEREO,
AudioFormat.ENCODING_PCM_16BIT);
this.bufferSize = (3 * (this.minBufferSize / 2)) & ~3;
this.audioTrack = new AudioTrack.Builder()

Let me know if you need any more help!

@Wikiz05
Copy link

Wikiz05 commented Mar 3, 2019

Hi I have the same issue could you please help me how to fix it?

@kevinstadler kevinstadler pinned this issue Mar 18, 2019
@kevinstadler kevinstadler changed the title Audio input on Android Add Android audio input support to JSynAndroidAudioDeviceManager Mar 18, 2019
@Calsign
Copy link
Contributor Author

Calsign commented May 23, 2019

I found JSyn's Android sample code. It is not encouraging:

https://github.com/philburk/jsyn/blob/17f8a07bbf81b5429b0db69766ffd024c892d1d7/android/com/jsyn/devices/android/AndroidAudioForJSyn.java#L155-L162

It says "JSyn audio input not implemented on Android.".

@Calsign
Copy link
Contributor Author

Calsign commented May 23, 2019

This should be working with Calsign/processing-sound@d3f973d. The only thing needed is setting the RECORD_AUDIO permission, which the AudioInput example does not currently do. I am looking into this now.

@Calsign
Copy link
Contributor Author

Calsign commented May 23, 2019

Created a new, Android-specific "AudioInputAndroid" example with Calsign/processing-sound@96eb20b.

I don't know of a way to use one example for both desktop and Android because Android requires requesting the RECORD_AUDIO permission, but desktop does not support Android mode's permission requesting functionality.

I think this is complete and will submit a PR pending approval from @kevinstadler.

kevinstadler added a commit that referenced this issue May 24, 2019
Support audio input on Android (implements #20)
@kevinstadler
Copy link
Collaborator

This looks fantastic, thank you very much!

One question, do you think it would be feasible to include the step of requesting Audio permission in the AudioIn class itself, i.e. so that the user doesn't have to do it themselves? Upon instantiation of AudioIn the constructor could check if the current audio engine is an instance of JsynAndroidAudioDeviceManager and if it is call the corresponding Android API to request permission. As long as the android.jar is present during building I don't think this Android-specific bit of code should cause any problems for Processing on desktop since the missing Android API methods won't ever actually get invoked there.

@kevinstadler
Copy link
Collaborator

I've looked into this a bit more:

  • checking whether the sketch is currently running on Android is easy enough, the most direct way to check/request the permission from within the library would be calling the corresponding Android ContextCompat methods, but I'm not sure where to get the required Android Context object from, maybe @Calsign knows anything about this?
  • a slightly more indirect but in the end equivalent way would be to use the sketch's own hasPermission() and requestPermission() methods from within the AudioIn class (basically what is happening in your example, only that the user doesn't have to do it themselves). This is in theory possible because the PApplet object that we need for this is passed to the AudioIn constructor, however because the processing-android sketch class is not its own class but rather a processing.core.PApplet substitute I'm not clear on how to get sound library code calling the android-mode specific hasPermission() or requestPermission() to compile. If I compiled the sound library against the android mode's core.jar (which has the PApplet.*permission() methods) this should work, and hopefully not wreak havoc on the 'normal' Processing because none of the other Android-PApplet-specific methods are called.

@kevinstadler
Copy link
Collaborator

@Calsign I have added some code to AudioIn which prints a warning message if a user attempts to capture audio on Android without having requested the appropriate permission first. I currently have no means to run this in Android mode, could you do me a favour and test it and tell me if it behaves as expected? You can simply replace your current sound.jar with this one (I know the zipping is superfluous but GitHub didn't allow upload of a straight-up jar, so just unzip it):
sound.jar.zip

Do you think the warning message as it is printed right now is meaningful/useful? What currently happens when the user instantiates AudioIn without the appropriate permission, does the sketch crash or does it just not capture any sound?

@kevinstadler
Copy link
Collaborator

Full library build also available from here now, but not yet pushed to Processing contribution manager: https://github.com/processing/processing-sound/releases/tag/v2.2.0

@Calsign
Copy link
Contributor Author

Calsign commented May 24, 2019

Quick note: It looks like the modified ant build script correctly sets up processing-core.zip, but still uses the regular core.jar unless that file is deleted. This was my experience anyway.

I don't think the current check is quite right. There are two components to requesting a permission, and they are both required, it is not one or the other: the permission must be specified in the manifest, and it must be granted at runtime by the user - this only has to happen the first time that the app is run, though, assuming that the user accepts the permission.

If the permission is not granted in the manifest and the sketch tries to use it (by creating the AudioRecord object), the sketch crashes with a stack trace that is not helpful to the average user.

If the permission is granted in the manifest but not at runtime, the sketch crashes with the same message. This happens even if the dialog is presented correctly but the user declines to grant the permission.

There is no way to avoid the user needing to specify the permission in the manifest (typically through the permissions selector dialog), I don't think it is possible for the library to add anything to the manifest. For this, I think the best we can do is print a warning telling the user to add the permission.

As for runtime permissions - like you said, Processing's requestPermissions() is asynchronous. I can think of potential ways to make the request synchronous instead (by blocking until the user makes a selection), but there would need to be a way to receive the permissions result request. Processing implements Android's onRequestPermissionsResult(), but I do not know a way of hooking into this functionality, and I don't think reflection is powerful enough to modify the behavior of that function without breaking other things.

I think the issues with runtime permission requests could be remedied if Processing's Android mode included a library callback method for onRequestPermissionsResult(), but this does not currently exist.

Other libraries, like Ketai, make no attempt to handle permissions for the user of the library. I think it would be nice to hide away this functionality, but it would create a lot of messiness that it is perhaps better to do without. There is a good explanation of how to manage permissions on the Android Processing site that users can be pointed to.

I think the best solution would be as follows: Check for permission access in AudioIn and print a warning message if it is not granted, as you have done already. That warning message should say that both the manifest and runtime requests are necessary. If the permission is not granted, in addition to printing the warning, don't initialize the AudioIn (to prevent crash). There are perhaps legitimate cases where the user does not want to grant the microphone permission, but there are still other features of the app that are still usable even with a silenced microphone input.

I will go ahead and implement this approach and test thoroughly. @kevinstadler let me know if this sounds reasonable or if there are further changes necessary.

@Calsign
Copy link
Contributor Author

Calsign commented May 24, 2019

@kevinstadler I am having difficulty figuring out what to do if the permission is not granted. There should be some way to provide a dummy microphone input for this case, but I am not sure how this should be done. The problem is that it is still crashing, and the warning message is not appearing, perhaps because the warning message is printed too close to the crash.

@kevinstadler
Copy link
Collaborator

Quick note: It looks like the modified ant build script correctly sets up processing-core.zip, but still uses the regular core.jar unless that file is deleted. This was my experience anyway.

Ah yes indeed, I didn't want to force people to re-download all dependencies every time so ant clean is a bit too lenient when it comes to binning old dependencies, I might have to set up the ant classpath more precisely...

As for runtime permissions - like you said, Processing's requestPermissions() is asynchronous. I can think of potential ways to make the request synchronous instead (by blocking until the user makes a selection), but there would need to be a way to receive the permissions result request.

Blocking until the user makes a selection sounds ideal to me, could we not just do another call to hasPermission() after the blocking call to see if the user has granted permission?

I will go ahead and implement this approach and test thoroughly. @kevinstadler let me know if this sounds reasonable or if there are further changes necessary.

Yes that all sounds good to me!

I am having difficulty figuring out what to do if the permission is not granted. There should be some way to provide a dummy microphone input for this case, but I am not sure how this should be done. The problem is that it is still crashing, and the warning message is not appearing, perhaps because the warning message is printed too close to the crash.

Hmmm, could you post the stack trace that results when the permission is not granted? If we find a way to check that permission has not been granted (either by calling hasPermission() again, or simply by catching the resulting exception) we could simply print a warning message and set some vital component of the AudioIn to null. This way the sketch will keep on running at first but then throw a NullPointerException as soon as the user actually tries to use the dysfunctional AudioIn, I think that's consistent with what would happen in other parts of the sound library.

@Calsign
Copy link
Contributor Author

Calsign commented May 27, 2019

I guess the best way to handle this is to just throw our own exception with a descriptive error message. I have implemented this in PR #36.

@kevinstadler
Copy link
Collaborator

That looks good indeed, thank you very much! I'll put the 2.2.0 build up later today, it might take another couple of days before it hits the Processing contribution manager to be available for everyone.

@kevinstadler kevinstadler unpinned this issue Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants