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

Initialize pa_buffer_attr.maxlength to -1 #102069

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

goatchurchprime
Copy link
Contributor

Fixes #102013 (and other long term unreliabilities of the pulseaudio microphone on Linux) by initializing and uninitialized buffer definition attribute to -1 as recommended here: https://maemo.org/api_refs/5.0/5.0-final/pulseaudio/structpa__buffer__attr.html

The other three attributes (tlength, prebuf, minreq) are not used by the recording feature pa_stream_connect_record() and probably don't have to be initialized.

@BastiaanOlij
Copy link
Contributor

BastiaanOlij commented Jan 27, 2025

Not an expert on the audio system so hesitant to do more than having a quick look. Reading the documentation, leaving this property uninitialized seems like a bad idea. Seeing they themselves recommend setting this to -1 so the buffer is allocated with the maximum supported size, this change seems sound.

@@ -726,6 +726,7 @@ Error AudioDriverPulseAudio::init_input_device() {
int input_buffer_size = input_buffer_frames * spec.channels;

pa_buffer_attr attr;
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest also zeroing the struct so at least we have sane defaults if we end up relying on some of the other struct fields.

Suggested change
pa_buffer_attr attr;
pa_buffer_attr attr = {};

@akien-mga akien-mga added this to the 4.4 milestone Jan 27, 2025
@akien-mga akien-mga added cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:3.6 Considered for cherry-picking into a future 3.6.x release labels Jan 27, 2025
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Great find! Thanks for looking into this.

@Repiteo Repiteo merged commit 07ceb0f into godotengine:master Jan 27, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 27, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release cherrypick:3.6 Considered for cherry-picking into a future 3.6.x release cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:audio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audio Mic Record Demo fails to record when debug/settings/stdout/verbose_stdout=true on Linux
4 participants