-
Notifications
You must be signed in to change notification settings - Fork 733
VIDEO-11239 Add Krisp Noise Cancellation #750
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
Conversation
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.
Thanks @olipyskoty! Looks good!
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.
Awesome work @manjeshbhargav!
Left one small comment but otherwise it LGTM!
noiseCancellationOptions: { | ||
sdkAssetsPath: '/noisecancellation', | ||
vendor: 'krisp', | ||
}, |
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.
It seems like we re-use this multiple times in this file. Can we store this as a variable once and reuse that?
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.
@PikaJoyce I'll do that in a separate PR. Now, I'm focused on getting this out.
Is there a possibility to disable this feature? I'm getting errors in the Firefox console, and am unable to join a room with version 0.9.0. |
Contributing to Twilio
Pull Request Details
JIRA link(s):
Description
This PR adds Krisp to the Ahoy app. If Krisp is installed, users can enable and disable noise cancellation by clicking the switch button on the Device Selection Screen before joining the room, and any time during the call by accessing Audio and Video settings.
Burndown
Before review
npm test
Before merge