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

ios microphone recording bugfix #41592

Closed
wants to merge 2 commits into from
Closed

Conversation

taeyoonwf
Copy link

@taeyoonwf taeyoonwf commented Aug 29, 2020

Fixes #33885

@Chaosus Chaosus added this to the 4.0 milestone Aug 29, 2020
@Calinou Calinou added cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:audio topic:porting labels Aug 29, 2020
@akien-mga
Copy link
Member

CC @bruvzg @naithar

@@ -142,7 +142,7 @@ Error AudioDriverCoreAudio::init() {

unsigned int buffer_size = buffer_frames * channels;
samples_in.resize(buffer_size);
input_buf.resize(buffer_size);
input_buf.resize(buffer_size * 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's to fix:

a "-50" error from AudioUnitRender in audio_driver_coreaudio.cpp whenever microphone input is enabled. This is because bufferList.mBuffers[0].mDataByteSize is too small, since there is a mismatch between the capture buffer format and the audio device.

Maybe buffer size should be calculated separately form output buffer size, without using output_latency and mix_rate, these properties are for output. Set to the fixed size or add another property?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your point. " * 2" looks definitely a magic number. I hope someone who knows well this audio part can help to fix it in the near future because I'm not an expert in regards to this. On the other hand, fortunately, this file only has to do with the iPhone devices and this change works well on them, so making changes little by little would be also great.

Copy link
Contributor

@naithar naithar Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, fortunately, this file only has to do with the iPhone devices and this change works well on them, so making changes little by little would be also great.

This file is also used for macOS, so I guess it should be tested there too just in case.

I also think it would be better to calculate input_buffer_size separately or make it into constant/define. This should make it easier to read and maintain.

Copy link
Author

@taeyoonwf taeyoonwf Sep 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert it and fix it later then. When I have some time to do it, I'll give it a try.

@akien-mga
Copy link
Member

Could you squash the two commits into one?

@taeyoonwf
Copy link
Author

I'm sorry, I started working for a game engine company this month, so I can't keep my contribution to any other game engines because it's dealt with outside activity. This change is necessary for iOS, so I believe someone can also contribute it instead of me.

@taeyoonwf taeyoonwf closed this Oct 13, 2020
@akien-mga akien-mga added archived salvageable and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AudioStreamMicrophone is broken on iOS hardware
7 participants