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 alternative for audio device switching and add flag to disable the use of WebAudio #37

Merged
merged 1 commit into from Dec 17, 2019

Conversation

XHatan
Copy link
Contributor

@XHatan XHatan commented Dec 15, 2019

Issue #:

Description of changes

We use WebAudio in DefaultDeviceController for audio stream/track management. It poses some problem: for example, in Safari, audio will stop when apps running in background.
This PR provides an alternative implementation where we use audio stream directly.
When switching device, replaceTrack is used in RTCRtpSender to avoid rejoining meeting since triggering a re-subscription violates certain server assumptions.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@andibrae-amzn
Copy link

Typo in commit message: "Add flag to disbale webaudio"

@XHatan XHatan changed the title Add flag to disbale webaudio Add alternative for audio device switching and add flag to disable the use of WebAudio Dec 16, 2019
andibrae-amzn
andibrae-amzn previously approved these changes Dec 17, 2019
/**
* Feature flag to enable WebAudio processing
*/
enableWebAudio: boolean = true;

Choose a reason for hiding this comment

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

I think we should set this to false by default so that it's opt-in rather than opt-out.

@andibrae-amzn andibrae-amzn dismissed their stale review December 17, 2019 00:48

Requested change

@andibrae-amzn
Copy link

I haven't seen the Travis build fail with that error before. Any idea what's going with that?

@XHatan
Copy link
Contributor Author

XHatan commented Dec 17, 2019

I haven't seen the Travis build fail with that error before. Any idea what's going with that?

I noticed when I built it locally, after all tests passed showing 1112 tests passing (31s), it took really long time (minutes) to show coverage summary. I think it's due to the tests in 'DefaultAudioVideoController' that I added.

@andibrae-amzn
Copy link

Typo in commit message "disbale" instead of "disable".

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.

None yet

2 participants