Skip to content

Conversation

@cloudwebrtc
Copy link
Contributor

@cloudwebrtc cloudwebrtc commented Jun 24, 2024

This parameter is used to avoid queues and silence frames causing latency accumulation, and is typically used when connecting to Audio Capture devices or other real-time sources.

@cloudwebrtc cloudwebrtc requested a review from theomonnom June 26, 2024 01:33
@cloudwebrtc cloudwebrtc changed the title Add disable-silent-frames for AudioSource. Add enable_queue for AudioSourceOptions. Jun 26, 2024
@cloudwebrtc cloudwebrtc merged commit fe6503b into main Jun 26, 2024
@cloudwebrtc cloudwebrtc deleted the duan/support-disable-silent-frames-for-audio-source branch June 26, 2024 07:11
Copy link
Contributor

@maxbrunsfeld maxbrunsfeld left a comment

Choose a reason for hiding this comment

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

This commit introduced a regression, causing distorted audio in our Rust app. The enable_queue option is not actually respected, due to a bug I mentioned above.

/cc @theomonnom @cloudwebrtc


let mut inner = self.inner.lock().await;
let mut samples = 0;
let enable_queue = self.sys_handle.audio_options().enable_queue;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this always returns false, because the enable_queue option is not initialized explicitly in the C++ implementation of this audio_options method.

Does the enable_queue bit even need to be passed into C++? Why not just store the enable_queue as a field on the rust NativeAudioSource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey, yes, there is a bug here, I created another PR to fix it
#360

Copy link
Contributor Author

@cloudwebrtc cloudwebrtc Jun 29, 2024

Choose a reason for hiding this comment

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

After modification, enable_queue is an optional parameter of NewAudioSourceRequest instead of AudioSourceOptions. and store the enable_queue field on the rust NativeAudioSource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants