-
-
Notifications
You must be signed in to change notification settings - Fork 23.8k
Implement permission request for Apple embedded platforms, fix microphone input #107233
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
Implement permission request for Apple embedded platforms, fix microphone input #107233
Conversation
that the OS will provide us, cope with the fact that iOS will adjust the number of frames dynamically based on your session type. This should fix godotengine#33885
|
This pull request now also includes a fix for #33885 |
|
@akien-mga Now that this went from 'necessary enhancement to fix a bug' to 'fixes a five year old bug that has prevented recording from working on iOS', could we change the label from enhancement? |
|
I wonder how #106305 has been confirmed to be working by its author, since I expect camera access to also require a permission just like microphone access. |
At least on macOS, camera server do auto request permission when feed is activated. |
I've created a test application where I've incorporated CameraTexture into the scene tree. As a result, you'll notice a camera access permission dialog appearing when the app launches. |
bruvzg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on 10. gen. iPad, recording seems to be working (note for testing, appropriate category should set in audio/general/ios/session_category project setting and permission description added in export config for it to work).
| bufferList.mBuffers[0].mDataByteSize = ad->input_buf.size() * sizeof(int16_t); | ||
|
|
||
| OSStatus result = AudioUnitRender(ad->input_unit, ioActionFlags, inTimeStamp, inBusNumber, inNumberFrames, &bufferList); | ||
| ad->input_buf.resize(inNumberFrames * ad->capture_channels); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inpuf_buf is not used for anything except temporary storage here, so if mData is nullptr, and the audio unit provide its own buffer it can be completely removed.
diff --git a/drivers/coreaudio/audio_driver_coreaudio.h b/drivers/coreaudio/audio_driver_coreaudio.h
index 64c77fe97f..36f5eee73b 100644
--- a/drivers/coreaudio/audio_driver_coreaudio.h
+++ b/drivers/coreaudio/audio_driver_coreaudio.h
@@ -59,7 +59,7 @@ class AudioDriverCoreAudio : public AudioDriver {
unsigned int capture_buffer_frames = 0;
Vector<int32_t> samples_in;
- Vector<int16_t> input_buf;
+ unsigned int buffer_size = 0;
#ifdef MACOS_ENABLED
PackedStringArray _get_device_list(bool capture = false);
diff --git a/drivers/coreaudio/audio_driver_coreaudio.mm b/drivers/coreaudio/audio_driver_coreaudio.mm
index fb870e4740..8763a1b933 100644
--- a/drivers/coreaudio/audio_driver_coreaudio.mm
+++ b/drivers/coreaudio/audio_driver_coreaudio.mm
@@ -243,10 +243,9 @@
bufferList.mNumberBuffers = 1;
bufferList.mBuffers[0].mData = nullptr;
bufferList.mBuffers[0].mNumberChannels = ad->capture_channels;
- bufferList.mBuffers[0].mDataByteSize = ad->input_buf.size() * sizeof(int16_t);
+ bufferList.mBuffers[0].mDataByteSize = ad->buffer_size * sizeof(int16_t);
OSStatus result = AudioUnitRender(ad->input_unit, ioActionFlags, inTimeStamp, inBusNumber, inNumberFrames, &bufferList);
- ad->input_buf.resize(inNumberFrames * ad->capture_channels);
if (result == noErr) {
int16_t *data = (int16_t *)bufferList.mBuffers[0].mData;
for (unsigned int i = 0; i < inNumberFrames * ad->capture_channels; i++) {
@@ -459,8 +458,7 @@
// Sample rate is independent of channels (ref: https://stackoverflow.com/questions/11048825/audio-sample-frequency-rely-on-channels)
capture_buffer_frames = closest_power_of_2(latency * (uint32_t)capture_mix_rate / (uint32_t)1000);
- unsigned int buffer_size = capture_buffer_frames * capture_channels;
- input_buf.resize(buffer_size);
+ buffer_size = capture_buffer_frames * capture_channels;
AURenderCallbackStruct callback;
memset(&callback, 0, sizeof(AURenderCallbackStruct));| get_main_loop()->emit_signal(SNAME("on_request_permissions_result"), p_name, granted); | ||
| }]; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }; | |
| } |
| - [code]OS.request_permission("appleembedded.permission.AUDIO_RECORD")[/code] | ||
| [b]Note:[/b] On Android, permission must be checked during export. | ||
| [b]Note:[/b] This method is implemented on Android and macOS. | ||
| [b]Note:[/b] This method is implemented on Android, macOS and visionOS platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [b]Note:[/b] This method is implemented on Android, macOS and visionOS platforms. | |
| [b]Note:[/b] This method is implemented on Android, macOS, and visionOS platforms. |
Please use Oxford comma.
| <description> | ||
| On Android devices: Returns the list of dangerous permissions that have been granted. | ||
| On macOS: Returns the list of granted permissions and user selected folders accessible to the application (sandboxed applications only). Use the native file dialog to request folder access permission. | ||
| On iOS, VisionOS: Returns the list of granted permissions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| On iOS, VisionOS: Returns the list of granted permissions. | |
| On iOS, visionOS: Returns the list of granted permissions. |
|
While addressing review comments, please also:
|
|
Folks, apologies, but I am away for a month on vacation. Would appreciate if someone takes over this patch. |
…nput Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
|
Superseded by #107973. Thanks for the contribution! |
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
…nput Co-Authored-By: Miguel de Icaza <[email protected]> Supersedes godotengine#107233 Fixes godotengine/godot-proposals#12563 Fixes godotengine#33885 Superseding Miguel's PR to get it in during the beta stage.
Fixes godotengine/godot-proposals#12563 but also fixes the "Recording audio on iOS does not work" from 33885.
This now addresses all issues I spotted in #33885 and I have tested it against the Godot recording sample and it finally works on iOS.