From c5c9ecdd87225ecd4978e9f6176a8a6d1fdd4252 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 16 Feb 2022 16:18:18 +0100 Subject: [PATCH] spa: improve the AEC interface Place the methods on the interface so that we can call them. Rename create to init because that is what it does. Add support for listener and events so that we can signal property changes later. --- spa/include/spa/interfaces/audio/aec.h | 71 +++++++++++++++++--- spa/plugins/aec/aec-null.c | 62 ++++++++--------- spa/plugins/aec/aec-webrtc.cpp | 54 ++++++--------- src/modules/module-echo-cancel.c | 39 ++++++----- src/modules/module-echo-cancel/echo-cancel.h | 34 ---------- 5 files changed, 130 insertions(+), 130 deletions(-) delete mode 100644 src/modules/module-echo-cancel/echo-cancel.h diff --git a/spa/include/spa/interfaces/audio/aec.h b/spa/include/spa/interfaces/audio/aec.h index e1fcda563..17e4e4e46 100644 --- a/spa/include/spa/interfaces/audio/aec.h +++ b/spa/include/spa/interfaces/audio/aec.h @@ -25,20 +25,71 @@ #include #include -#include #include -#include -#define SPA_TYPE_INTERFACE_AEC SPA_TYPE_INFO_INTERFACE_BASE "AEC" -#define SPA_VERSION_AUDIO_AEC 1 +#ifndef SPA_AUDIO_AEC_H +#define SPA_AUDIO_AEC_H -struct echo_cancel_info { +#ifdef __cplusplus +extern "C" { +#endif + +#define SPA_TYPE_INTERFACE_AUDIO_AEC SPA_TYPE_INFO_INTERFACE_BASE "Audio:AEC" + +#define SPA_VERSION_AUDIO_AEC 0 +struct spa_audio_aec { struct spa_interface iface; const char *name; - const struct spa_dict info; + const struct spa_dict *info; const char *latency; - int (*create) (struct spa_handle *handle, const struct spa_dict *args, const struct spa_audio_info_raw *info); - int (*run) (struct spa_handle *handle, const float *rec[], const float *play[], float *out[], uint32_t n_samples); - struct spa_dict *(*get_properties) (struct spa_handle *handle); - int (*set_properties) (struct spa_handle *handle, const struct spa_dict *args); }; + +struct spa_audio_aec_info { +#define SPA_AUDIO_AEC_CHANGE_MASK_PROPS (1u<<0) + uint64_t change_mask; + + const struct spa_dict *props; +}; + +struct spa_audio_aec_events { +#define SPA_VERSION_AUDIO_AEC_EVENTS 0 + uint32_t version; /**< version of this structure */ + + /** Emitted when info changes */ + void (*info) (void *data, const struct spa_audio_aec_info *info); +}; + +struct spa_audio_aec_methods { +#define SPA_VERSION_AUDIO_AEC_METHODS 0 + uint32_t version; + + int (*add_listener) (void *object, + struct spa_hook *listener, + const struct spa_audio_aec_events *events, + void *data); + + int (*init) (void *data, const struct spa_dict *args, const struct spa_audio_info_raw *info); + int (*run) (void *data, const float *rec[], const float *play[], float *out[], uint32_t n_samples); + int (*set_props) (void *data, const struct spa_dict *args); +}; + +#define spa_audio_aec_method(o,method,version,...) \ +({ \ + int _res = -ENOTSUP; \ + struct spa_audio_aec *_o = o; \ + spa_interface_call_res(&_o->iface, \ + struct spa_audio_aec_methods, _res, \ + method, version, ##__VA_ARGS__); \ + _res; \ +}) + +#define spa_audio_aec_add_listener(o,...) spa_audio_aec_method(o, add_listener, 0, __VA_ARGS__) +#define spa_audio_aec_init(o,...) spa_audio_aec_method(o, init, 0, __VA_ARGS__) +#define spa_audio_aec_run(o,...) spa_audio_aec_method(o, run, 0, __VA_ARGS__) +#define spa_audio_aec_set_props(o,...) spa_audio_aec_method(o, set_props, 0, __VA_ARGS__) + +#ifdef __cplusplus +} /* extern "C" */ +#endif + +#endif /* SPA_AUDIO_AEC_H */ diff --git a/spa/plugins/aec/aec-null.c b/spa/plugins/aec/aec-null.c index 3168a6b36..2909eb213 100644 --- a/spa/plugins/aec/aec-null.c +++ b/spa/plugins/aec/aec-null.c @@ -30,7 +30,11 @@ struct impl { struct spa_handle handle; + struct spa_audio_aec aec; struct spa_log *log; + + struct spa_hook_list hooks_list; + uint32_t channels; }; @@ -38,54 +42,38 @@ static struct spa_log_topic log_topic = SPA_LOG_TOPIC(0, "spa.aec.null"); #undef SPA_LOG_TOPIC_DEFAULT #define SPA_LOG_TOPIC_DEFAULT &log_topic -static int null_create(struct spa_handle *handle, const struct spa_dict *args, const struct spa_audio_info_raw *info) +static int null_init(void *data, const struct spa_dict *args, const struct spa_audio_info_raw *info) { - struct impl *impl; - impl = (struct impl *) handle; + struct impl *impl = data; impl->channels = info->channels; - return 0; } -static int null_run(struct spa_handle *handle, const float *rec[], const float *play[], float *out[], uint32_t n_samples) +static int null_run(void *data, const float *rec[], const float *play[], float *out[], uint32_t n_samples) { - struct impl *impl = (struct impl *) handle; + struct impl *impl = data; uint32_t i; for (i = 0; i < impl->channels; i++) memcpy(out[i], rec[i], n_samples * sizeof(float)); return 0; } -struct spa_dict *null_get_properties(SPA_UNUSED struct spa_handle *handle) -{ - /* Not supported */ - return NULL; -} - -int null_set_properties(SPA_UNUSED struct spa_handle *handle, SPA_UNUSED const struct spa_dict *args) -{ - /* Not supported */ - return -1; -} - -static struct echo_cancel_info echo_cancel_null_impl = { - .name = "null", - .info = SPA_DICT_INIT(NULL, 0), - .latency = NULL, - .create = null_create, +static struct spa_audio_aec_methods impl_aec = { + .init = null_init, .run = null_run, - .get_properties = null_get_properties, - .set_properties = null_set_properties, }; static int impl_get_interface(struct spa_handle *handle, const char *type, void **interface) { + struct impl *impl; spa_return_val_if_fail(handle != NULL, -EINVAL); spa_return_val_if_fail(interface != NULL, -EINVAL); - if (spa_streq(type, SPA_TYPE_INTERFACE_AEC)) - *interface = &echo_cancel_null_impl; + impl = (struct impl *) handle; + + if (spa_streq(type, SPA_TYPE_INTERFACE_AUDIO_AEC)) + *interface = &impl->aec; else return -ENOENT; @@ -118,23 +106,29 @@ impl_init(const struct spa_handle_factory *factory, spa_return_val_if_fail(factory != NULL, -EINVAL); spa_return_val_if_fail(handle != NULL, -EINVAL); - echo_cancel_null_impl.iface = SPA_INTERFACE_INIT( - SPA_TYPE_INTERFACE_AEC, - SPA_VERSION_AUDIO_AEC, - NULL, - NULL); - handle->get_interface = impl_get_interface; handle->clear = impl_clear; + impl = (struct impl *) handle; + + impl->aec.iface = SPA_INTERFACE_INIT( + SPA_TYPE_INTERFACE_AUDIO_AEC, + SPA_VERSION_AUDIO_AEC, + &impl_aec, impl); + impl->aec.name = "null"; + impl->aec.info = NULL; + impl->aec.latency = NULL; + impl->log = (struct spa_log*)spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log); spa_log_topic_init(impl->log, &log_topic); + spa_hook_list_init(&impl->hooks_list); + return 0; } static const struct spa_interface_info impl_interfaces[] = { - {SPA_TYPE_INTERFACE_AEC,}, + {SPA_TYPE_INTERFACE_AUDIO_AEC,}, }; static int diff --git a/spa/plugins/aec/aec-webrtc.cpp b/spa/plugins/aec/aec-webrtc.cpp index d44fa6e30..59c910afc 100644 --- a/spa/plugins/aec/aec-webrtc.cpp +++ b/spa/plugins/aec/aec-webrtc.cpp @@ -38,6 +38,8 @@ struct impl_data { struct spa_handle handle; + struct spa_audio_aec aec; + struct spa_log *log; std::unique_ptr apm; spa_audio_info_raw info; @@ -58,9 +60,9 @@ static bool webrtc_get_spa_bool(const struct spa_dict *args, const char *key, bo return value; } -static int webrtc_create(struct spa_handle *handle, const struct spa_dict *args, const struct spa_audio_info_raw *info) +static int webrtc_init(void *data, const struct spa_dict *args, const struct spa_audio_info_raw *info) { - auto impl = reinterpret_cast(handle); + auto impl = reinterpret_cast(data); bool extended_filter = webrtc_get_spa_bool(args, "webrtc.extended_filter", true); bool delay_agnostic = webrtc_get_spa_bool(args, "webrtc.delay_agnostic", true); @@ -120,9 +122,9 @@ static int webrtc_create(struct spa_handle *handle, const struct spa_dict *args, return 0; } -static int webrtc_run(struct spa_handle *handle, const float *rec[], const float *play[], float *out[], uint32_t n_samples) +static int webrtc_run(void *data, const float *rec[], const float *play[], float *out[], uint32_t n_samples) { - auto impl = reinterpret_cast(handle); + auto impl = reinterpret_cast(data); webrtc::StreamConfig config = webrtc::StreamConfig(impl->info.rate, impl->info.channels, false); unsigned int num_blocks = n_samples * 1000 / impl->info.rate / 10; @@ -158,36 +160,20 @@ static int webrtc_run(struct spa_handle *handle, const float *rec[], const float return 0; } -struct spa_dict *webrtc_get_properties(SPA_UNUSED struct spa_handle *handle) -{ - /* Not supported */ - return NULL; -} - -int webrtc_set_properties(SPA_UNUSED struct spa_handle *handle, SPA_UNUSED const struct spa_dict *args) -{ - /* Not supported */ - return -1; -} - -static struct echo_cancel_info echo_cancel_webrtc_impl = { - .name = "webrtc", - .info = SPA_DICT_INIT(NULL, 0), - .latency = "480/48000", - .create = webrtc_create, +static struct spa_audio_aec_methods impl_aec = { + .init = webrtc_init, .run = webrtc_run, - .get_properties = webrtc_get_properties, - .set_properties = webrtc_set_properties, }; static int impl_get_interface(struct spa_handle *handle, const char *type, void **interface) { + auto impl = reinterpret_cast(handle); spa_return_val_if_fail(handle != NULL, -EINVAL); spa_return_val_if_fail(interface != NULL, -EINVAL); - if (spa_streq(type, SPA_TYPE_INTERFACE_AEC)) - *interface = &echo_cancel_webrtc_impl; + if (spa_streq(type, SPA_TYPE_INTERFACE_AUDIO_AEC)) + *interface = &impl->aec; else return -ENOENT; @@ -219,15 +205,19 @@ impl_init(const struct spa_handle_factory *factory, spa_return_val_if_fail(factory != NULL, -EINVAL); spa_return_val_if_fail(handle != NULL, -EINVAL); - echo_cancel_webrtc_impl.iface = SPA_INTERFACE_INIT( - SPA_TYPE_INTERFACE_AEC, - SPA_VERSION_AUDIO_AEC, - NULL, - NULL); - auto impl = new (handle) impl_data(); + impl->handle.get_interface = impl_get_interface; impl->handle.clear = impl_clear; + + impl->aec.iface = SPA_INTERFACE_INIT( + SPA_TYPE_INTERFACE_AUDIO_AEC, + SPA_VERSION_AUDIO_AEC, + &impl_aec, impl); + impl->aec.name = "webrtc", + impl->aec.info = NULL; + impl->aec.latency = "480/48000", + impl->log = (struct spa_log*)spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log); spa_log_topic_init(impl->log, &log_topic); @@ -235,7 +225,7 @@ impl_init(const struct spa_handle_factory *factory, } static const struct spa_interface_info impl_interfaces[] = { - {SPA_TYPE_INTERFACE_AEC,}, + {SPA_TYPE_INTERFACE_AUDIO_AEC,}, }; static int diff --git a/src/modules/module-echo-cancel.c b/src/modules/module-echo-cancel.c index 728325938..c933a9376 100644 --- a/src/modules/module-echo-cancel.c +++ b/src/modules/module-echo-cancel.c @@ -24,7 +24,6 @@ */ #include "config.h" -#include "module-echo-cancel/echo-cancel.h" #include #include @@ -188,7 +187,7 @@ struct impl { uint32_t out_ringsize; struct spa_ringbuffer out_ring; - const struct echo_cancel_info *aec_info; + struct spa_audio_aec *aec; uint32_t aec_blocksize; unsigned int capture_ready:1; @@ -288,7 +287,7 @@ static void process(struct impl *impl) pw_stream_queue_buffer(impl->playback, pout); /* Now run the canceller */ - echo_cancel_run(impl->aec_info, impl->spa_handle, rec, play_delayed, out, size / sizeof(float)); + spa_audio_aec_run(impl->aec, rec, play_delayed, out, size / sizeof(float)); /* Next, copy over the output to the output ringbuffer */ avail = spa_ringbuffer_get_write_index(&impl->out_ring, &oindex); @@ -652,8 +651,8 @@ static int setup_streams(struct impl *impl) pw_properties_set(props, PW_KEY_NODE_LINK_GROUP, str); if ((str = pw_properties_get(impl->source_props, PW_KEY_NODE_LATENCY)) != NULL) pw_properties_set(props, PW_KEY_NODE_LATENCY, str); - else if (impl->aec_info->latency) - pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec_info->latency); + else if (impl->aec->latency) + pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec->latency); impl->capture = pw_stream_new(impl->core, "Echo-Cancel Capture", props); @@ -685,8 +684,8 @@ static int setup_streams(struct impl *impl) pw_properties_set(props, PW_KEY_NODE_LINK_GROUP, str); if ((str = pw_properties_get(impl->sink_props, PW_KEY_NODE_LATENCY)) != NULL) pw_properties_set(props, PW_KEY_NODE_LATENCY, str); - else if (impl->aec_info->latency) - pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec_info->latency); + else if (impl->aec->latency) + pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec->latency); impl->playback = pw_stream_new(impl->core, "Echo-Cancel Playback", props); @@ -999,42 +998,42 @@ int pipewire__module_init(struct pw_impl_module *module, const char *args) return -ENOENT; } - if ((res = spa_handle_get_interface(handle, SPA_TYPE_INTERFACE_AEC, &iface)) < 0) { - pw_log_error("can't get %s interface %d", SPA_TYPE_INTERFACE_AEC, res); + if ((res = spa_handle_get_interface(handle, SPA_TYPE_INTERFACE_AUDIO_AEC, &iface)) < 0) { + pw_log_error("can't get %s interface %d", SPA_TYPE_INTERFACE_AUDIO_AEC, res); return res; } - impl->aec_info = iface; + impl->aec = iface; impl->spa_handle = handle; - if (impl->aec_info->iface.version != SPA_VERSION_AUDIO_AEC) { + if (impl->aec->iface.version != SPA_VERSION_AUDIO_AEC) { pw_log_error("codec plugin %s has incompatible ABI version (%d != %d)", - SPA_NAME_AEC, impl->aec_info->iface.version, SPA_VERSION_AUDIO_AEC); + SPA_NAME_AEC, impl->aec->iface.version, SPA_VERSION_AUDIO_AEC); res = -ENOENT; goto error; } - (void)SPA_SUPPORT_INIT(SPA_TYPE_INTERFACE_AEC, (struct echo_cancel_info *)impl->aec_info); + (void)SPA_SUPPORT_INIT(SPA_TYPE_INTERFACE_AUDIO_AEC, (struct spa_audio_aec *)impl->aec); - pw_log_info("Using plugin AEC %s", impl->aec_info->name); + pw_log_info("Using plugin AEC %s", impl->aec->name); if ((str = pw_properties_get(props, "aec.args")) != NULL) aec_props = pw_properties_new_string(str); else aec_props = pw_properties_new(NULL, NULL); - if (echo_cancel_create(impl->aec_info, impl->spa_handle, &aec_props->dict, &impl->info)) { - pw_log_error("codec plugin %s create failed", impl->aec_info->name); + if (spa_audio_aec_init(impl->aec, &aec_props->dict, &impl->info)) { + pw_log_error("codec plugin %s create failed", impl->aec->name); res = -ENOENT; goto error; } pw_properties_free(aec_props); - if (impl->aec_info->latency) { + if (impl->aec->latency) { unsigned int num, denom, req_num, req_denom; unsigned int factor = 0; unsigned int new_num = 0; - spa_assert_se(sscanf(impl->aec_info->latency, "%u/%u", &num, &denom) == 2); + spa_assert_se(sscanf(impl->aec->latency, "%u/%u", &num, &denom) == 2); if ((str = pw_properties_get(props, PW_KEY_NODE_LATENCY)) != NULL) { sscanf(str, "%u/%u", &req_num, &req_denom); @@ -1043,8 +1042,8 @@ int pipewire__module_init(struct pw_impl_module *module, const char *args) } if (factor == 0 || new_num == 0) { - pw_log_info("Setting node latency to %s", impl->aec_info->latency); - pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec_info->latency); + pw_log_info("Setting node latency to %s", impl->aec->latency); + pw_properties_set(props, PW_KEY_NODE_LATENCY, impl->aec->latency); impl->aec_blocksize = sizeof(float) * impl->info.rate * num / denom; } else { pw_log_info("Setting node latency to %u/%u", new_num, req_denom); diff --git a/src/modules/module-echo-cancel/echo-cancel.h b/src/modules/module-echo-cancel/echo-cancel.h deleted file mode 100644 index ac83f70e4..000000000 --- a/src/modules/module-echo-cancel/echo-cancel.h +++ /dev/null @@ -1,34 +0,0 @@ -/* PipeWire - * - * Copyright © 2021 Wim Taymans - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - */ - - -#include -#include -#include -#include - -#include - -#define echo_cancel_create(i,...) (i)->create(__VA_ARGS__) -#define echo_cancel_run(i,...) (i)->run(__VA_ARGS__)