Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1162)

Unified Diff: content/renderer/media/media_stream_dependency_factory.cc

Issue 99033003: Enable platform echo cancellation through the AudioRecord path. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add PlatformEffects, unittests and clean up. Created 7 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/renderer/media/media_stream_dependency_factory.cc
diff --git a/content/renderer/media/media_stream_dependency_factory.cc b/content/renderer/media/media_stream_dependency_factory.cc
index 6172e06a8e31da352c32b056fc1cf6585ef2dcc6..f5995e16a2b6f84aa79883d9b8f409debe2892f1 100644
--- a/content/renderer/media/media_stream_dependency_factory.cc
+++ b/content/renderer/media/media_stream_dependency_factory.cc
@@ -96,6 +96,55 @@ void ApplyFixedAudioConstraints(RTCMediaConstraints* constraints) {
}
}
+typedef media::AudioParameters::PlatformEffects PlatformEffects;
tommi (sloooow) - chröme 2013/12/11 12:14:36 Is it a good idea to create a new type for this?
ajm 2013/12/12 01:51:27 Removed.
+
+// |key| is the constraint for which |constraints| should be updated, and
+// |effect| is the state of the corresponding platform effect.
tommi (sloooow) - chröme 2013/12/11 12:14:36 This function is still confusing to me since both
ajm 2013/12/11 17:46:49 That works, and I agree is much cleaner.
ajm 2013/12/12 01:51:27 Ugh, on further reflection, it doesn't work. First
+// |constraints| and |effect| are both input and output parameters.
+//
+// See the function below for a full description of the purpose.
+void ReconcileConstraintAndPlatformEffect(const char* key,
+ RTCMediaConstraints* constraints,
+ bool* effect) {
+ bool value;
+ if (!webrtc::FindConstraint(constraints, key, &value, NULL) ||
tommi (sloooow) - chröme 2013/12/11 12:14:36 nit: this fits on one line: if (!webrtc::FindCon
ajm 2013/12/12 01:51:27 Removed.
+ value == false) {
+ // If the constraint does not exist, or is set to false, disable the
+ // corresponding platform effect.
+ *effect = false;
+ DVLOG(1) << "Disabling platform effect: " << key;
+ } else if (*effect) {
+ // If the constraint is set to true, and the corresponding platform effect
+ // is available, disable the constraint.
+ constraints->AddMandatory(key,
+ webrtc::MediaConstraintsInterface::kValueFalse, true);
+ DVLOG(1) << "Disabling constraint: " << key;
+ }
+}
+
+// Updates |constraints| and |effects| to be consistent with each other. At
+// input, |constraints| must be the media constraints requested for the gUM
+// stream. At output |constraints| will be updated to reflect what should be
+// applied to the source (e.g. PeerConnection). At input, |effects| must be the
+// available platform (i.e. built-in, typically hardware) audio effects which
+// should be used in preference to software effects. At output, |effects| will
+// be updated to reflect what should be applied to the platform capturer.
+//
+// In particular this means:
+// - Any true constraint with an available corresponding platform effect at
+// input will be set to false at output.
+// - Any false constraint at input will cause the corresponding platform effect
+// to be disabled at output.
+void ReconcileConstraintsAndPlatformEffects(RTCMediaConstraints* constraints,
+ PlatformEffects* effects) {
+ // This currently only deals with AEC, but is prepared for expansion to other
+ // platform effects.
+ ReconcileConstraintAndPlatformEffect(
+ webrtc::MediaConstraintsInterface::kEchoCancellation,
+ constraints,
+ &effects->echo_canceller);
+}
+
class P2PPortAllocatorFactory : public webrtc::PortAllocatorFactoryInterface {
public:
P2PPortAllocatorFactory(
@@ -323,7 +372,16 @@ void MediaStreamDependencyFactory::CreateNativeMediaSources(
// TODO(xians): Create a new capturer for difference microphones when we
// support multiple microphones. See issue crbug/262117 .
- const StreamDeviceInfo device_info = source_data->device_info();
+ StreamDeviceInfo device_info = source_data->device_info();
+
+ RTCMediaConstraints platform_constraints = native_audio_constraints;
+ // This has the potential to update both the constraints and the platform
+ // effects, such that we prefer platform to software effects but disable
+ // platform effects when requested by constraints.
+ //
+ // See the function documentation for a full description.
+ ReconcileConstraintsAndPlatformEffects(&platform_constraints,
+ &device_info.device.input.effects);
scoped_refptr<WebRtcAudioCapturer> capturer(
MaybeCreateAudioCapturer(render_view_id, device_info));
if (!capturer.get()) {
@@ -341,7 +399,7 @@ void MediaStreamDependencyFactory::CreateNativeMediaSources(
// Creates a LocalAudioSource object which holds audio options.
// TODO(xians): The option should apply to the track instead of the source.
source_data->SetLocalAudioSource(
- CreateLocalAudioSource(&native_audio_constraints).get());
+ CreateLocalAudioSource(&platform_constraints).get());
source_observer->AddSource(source_data->local_audio_source());
}
@@ -910,7 +968,8 @@ MediaStreamDependencyFactory::MaybeCreateAudioCapturer(
device_info.session_id,
device_info.device.id,
device_info.device.matched_output.sample_rate,
- device_info.device.matched_output.frames_per_buffer)) {
+ device_info.device.matched_output.frames_per_buffer,
+ device_info.device.input.effects)) {
return NULL;
}

Powered by Google App Engine
This is Rietveld 408576698