[pulseaudio-discuss] [PATCH] echo-cancel: Convert AGC API to deal with pa_volume_t

[arun at accosted.net]

From: Arun Raghavan <git at arunraghavan.net>

It is expected that the underlying AGC mechanism will likely provide a
single volume for the source rather than a per-channel volume. Dealing
with per-channel volumes just adds complexity with regards to the
actual volume setting (depending on whether volume sharing is enabled or
not, we would set the volume on the source output of the virtual source,
and their sample specs may be different).

Using a single volume maps allows us to sidestep this problem entirely.
---
 src/modules/echo-cancel/echo-cancel.h        |  4 ++--
 src/modules/echo-cancel/module-echo-cancel.c | 32 +++++++++++++++-------------
 src/modules/echo-cancel/webrtc.cc            | 11 ++++------
 3 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/src/modules/echo-cancel/echo-cancel.h b/src/modules/echo-cancel/echo-cancel.h
index 83f8b85..ee67949 100644
--- a/src/modules/echo-cancel/echo-cancel.h
+++ b/src/modules/echo-cancel/echo-cancel.h
@@ -132,8 +132,8 @@ struct pa_echo_canceller {
 };
 
 /* Functions to be used by the canceller analog gain control routines */
-void pa_echo_canceller_get_capture_volume(pa_echo_canceller *ec, pa_cvolume *v);
-void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_cvolume *v);
+pa_volume_t pa_echo_canceller_get_capture_volume(pa_echo_canceller *ec);
+void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_volume_t volume);
 
 /* Computes EC block size in frames (rounded down to nearest power-of-2) based
  * on sample rate and milliseconds. */
diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
index 4e898d0..56adf8f 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -1535,12 +1535,16 @@ static int canceller_process_msg_cb(pa_msgobject *o, int code, void *userdata, i
 
     switch (code) {
         case ECHO_CANCELLER_MESSAGE_SET_VOLUME: {
-            pa_cvolume *v = (pa_cvolume *) userdata;
-
-            if (u->use_volume_sharing)
-                pa_source_set_volume(u->source, v, true, false);
-            else
-                pa_source_output_set_volume(u->source_output, v, false, true);
+            pa_volume_t v = PA_PTR_TO_UINT(userdata);
+            pa_cvolume vol;
+
+            if (u->use_volume_sharing) {
+                pa_cvolume_set(&vol, u->source->sample_spec.channels, v);
+                pa_source_set_volume(u->source, &vol, true, false);
+            } else {
+                pa_cvolume_set(&vol, u->source_output->sample_spec.channels, v);
+                pa_source_output_set_volume(u->source_output, &vol, false, true);
+            }
 
             break;
         }
@@ -1554,22 +1558,20 @@ static int canceller_process_msg_cb(pa_msgobject *o, int code, void *userdata, i
 }
 
 /* Called by the canceller, so source I/O thread context. */
-void pa_echo_canceller_get_capture_volume(pa_echo_canceller *ec, pa_cvolume *v) {
+pa_volume_t pa_echo_canceller_get_capture_volume(pa_echo_canceller *ec) {
 #ifndef ECHO_CANCEL_TEST
-    *v = ec->msg->userdata->thread_info.current_volume;
+    return pa_cvolume_avg(&ec->msg->userdata->thread_info.current_volume);
 #else
-    pa_cvolume_set(v, 1, PA_VOLUME_NORM);
+    return PA_VOLUME_NORM;
 #endif
 }
 
 /* Called by the canceller, so source I/O thread context. */
-void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_cvolume *v) {
+void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_volume_t v) {
 #ifndef ECHO_CANCEL_TEST
-    if (!pa_cvolume_equal(&ec->msg->userdata->thread_info.current_volume, v)) {
-        pa_cvolume *vol = pa_xnewdup(pa_cvolume, v, 1);
-
-        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, vol, 0, NULL,
-                pa_xfree);
+    if (pa_cvolume_avg(&ec->msg->userdata->thread_info.current_volume) != v) {
+        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, PA_UINT_TO_PTR(v),
+                0, NULL, NULL);
     }
 #endif
 }
diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
index 12bb556..f8aee33 100644
--- a/src/modules/echo-cancel/webrtc.cc
+++ b/src/modules/echo-cancel/webrtc.cc
@@ -537,9 +537,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out
     pa_deinterleave(rec, (void **) buf, rec_ss->channels, pa_sample_size(rec_ss), n);
 
     if (ec->params.webrtc.agc) {
-        pa_cvolume_init(&v);
-        pa_echo_canceller_get_capture_volume(ec, &v);
-        old_volume = webrtc_volume_from_pa(pa_cvolume_avg(&v));
+        pa_volume_t v = pa_echo_canceller_get_capture_volume(ec);
+        old_volume = webrtc_volume_from_pa(v);
         apm->gain_control()->set_stream_analog_level(old_volume);
     }
 
@@ -558,10 +557,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out
             new_volume = apm->gain_control()->stream_analog_level();
         }
 
-        if (old_volume != new_volume) {
-            pa_cvolume_set(&v, rec_ss->channels, webrtc_volume_to_pa(new_volume));
-            pa_echo_canceller_set_capture_volume(ec, &v);
-        }
+        if (old_volume != new_volume)
+            pa_echo_canceller_set_capture_volume(ec, webrtc_volume_to_pa(new_volume));
     }
 
     pa_interleave((const void **) buf, out_ss->channels, out, pa_sample_size(out_ss), n);
-- 
2.5.0