audio: Implement applyConstraints() plumbing for voice isolation [chromium/src : main]
Tomas Lundqvist (Gerrit)
Tomas Lundqvist added 2 comments![]( "Open in Gerrit")
SetVoiceIsolationEnabled(bool enabled);Tomas LundqvistSetVoiceIsolation(bool enabled)? Also drop "audio processor" (it's AudioProcessorControls)
Done
SelectSettingsAudioCapture(audio_source, current_request_->Constraints());Guido UrdanetaAll the work is happening here - don't we need to modify anything in it?
Tomas LundqvistWe probably need to modify SelectSettingsAudioCapture to allow it to change voice isolation.
Also, we need to ensure that we don't contradict other tracks that can have a contradicting exact constraints. The best way is probably to do it before calling SelectSettings and passing a list of allowed values from other tracks.
We need unit tests that cover this too.
Yes, we absolutely need to change SelectSettingsAudioCapture, but the changes there are in the constraints handling that is also used for getUserMedia(), and they are independent of this applyConstraints() plumbing. So it doesn't make sense to have them in an applyConstratins() cl. I collected those changes in the separate https://chromium-review.git.corp.google.com/c/chromium/src/+/7921250.
Related details
- Guido Urdaneta
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tomas Lundqvist (Gerrit)
Tomas Lundqvist voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tomas Lundqvist
| Auto-Submit | +1 |
1 comment
Tomas LundqvistThis method for disabling and reenabling voice isolation is documented in our DD: . But we are worried that it is maybe not this easy and would like to discuss the approach now rather than later. This just the plumbing. Note that it assumes that SelectSettingsAudioCapture does the right things when it comes to all settings, including voice isolation.
Olga SharonovaDD: https://docs.google.com/document/d/1qJRe4w5kSrkK3EJXJa8PF64wnNLKAmfD17j7Jo5tfzI/edit?tab=t.xbncevc0gupf#heading=h.2m5naq14ro0s and in the implementation section.
Tomas LundqvistAdd it to the CL description?
Also: describe applyConstraints behavior for voiceIsolation? For voiceIsolation in combination with other constraints?
Acknowledged
Related details
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tomas Lundqvist (Gerrit)
Tomas Lundqvist voted![]( "Open in Gerrit")
- Chromium IPC Reviews
- Olga Sharonova
- Olga Sharonova
- Tomas Lundqvist
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: kin...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): kin...@chromium.org
Reviewer source(s):
kin...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Kinuko Yasuda
- Olga Sharonova
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Sharonova (Gerrit)
Olga Sharonova added 1 comment![]( "Open in Gerrit")
GetVoiceIsolationHandler(output_params_, deliver_callback_.Get()));I'm wondering - Can we create a handler with a mock media::VoiceIsolation implementation? Then we can just verify the calls, and won't need to run tflight in tests. And it will make creating other tests easier as well.
Related details
- Kinuko Yasuda
- Olga Sharonova
- Tomas Lundqvist
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tomas Lundqvist (Gerrit)
Related details
- Kinuko Yasuda
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tomas Lundqvist (Gerrit)
Tomas Lundqvist voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Related details
- Chromium IPC Reviews
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: cl...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): cl...@chromium.org
Reviewer source(s):
cl...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Camille Lamy
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |