On 16 January 2016 at 14:03, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Wed, 2015-12-16 at 09:09 +0530, arun at accosted.net wrote:
>> From: Arun Raghavan <git at arunraghavan.net>
>>
>> ---
>> configure.ac | 2 +-
>> src/Makefile.am | 2 +-
>> src/modules/echo-cancel/webrtc.cc | 54 +++++++++++++++++++++------------------
>> 3 files changed, 31 insertions(+), 27 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index b9cd3d1..26c3e29 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -1371,7 +1371,7 @@ AC_ARG_ENABLE([webrtc-aec],
>> AS_HELP_STRING([--enable-webrtc-aec], [Enable the optional WebRTC-based echo canceller]))
>>
>> AS_IF([test "x$enable_webrtc_aec" != "xno"],
>> - [PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing ], [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])],
>> + [PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing > 0.1 ], [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])],
>> [HAVE_WEBRTC=0])
>>
>> AS_IF([test "x$enable_webrtc_aec" = "xyes" && test "x$HAVE_WEBRTC" = "x0"],
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index f1bd38d..533b646 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -50,7 +50,7 @@ AM_CPPFLAGS = \
>> -DPULSE_LOCALEDIR=\"$(localedir)\"
>> AM_CFLAGS = \
>> $(PTHREAD_CFLAGS)
>> -AM_CXXFLAGS = $(AM_CFLAGS)
>> +AM_CXXFLAGS = $(AM_CFLAGS) -std=c++11
>> SERVER_CFLAGS = -D__INCLUDED_FROM_PULSE_AUDIO
>>
>> AM_LIBADD = $(PTHREAD_LIBS) $(INTLLIBS)
>> diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
>> index 511c7ee..4a23377 100644
>> --- a/src/modules/echo-cancel/webrtc.cc
>> +++ b/src/modules/echo-cancel/webrtc.cc
>> @@ -33,8 +33,8 @@ PA_C_DECL_BEGIN
>> #include "echo-cancel.h"
>> PA_C_DECL_END
>>
>> -#include
>> -#include
>> +#include
>> +#include
>>
>> #define BLOCK_SIZE_US 10000
>>
>> @@ -80,6 +80,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>> pa_sample_spec *out_ss, pa_channel_map *out_map,
>> uint32_t *nframes, const char *args) {
>> webrtc::AudioProcessing *apm = NULL;
>> + webrtc::ProcessingConfig pconfig;
>> bool hpf, ns, agc, dgc, mobile, cn;
>> int rm = -1;
>> pa_modargs *ma;
>> @@ -153,7 +154,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>> }
>> }
>>
>> - apm = webrtc::AudioProcessing::Create(0);
>> + apm = webrtc::AudioProcessing::Create();
>>
>> out_ss->format = PA_SAMPLE_S16NE;
>> *play_ss = *out_ss;
>> @@ -163,22 +164,19 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>> *rec_ss = *out_ss;
>> *rec_map = *out_map;
>>
>> - apm->set_sample_rate_hz(out_ss->rate);
>> -
>> - apm->set_num_channels(out_ss->channels, out_ss->channels);
>> - apm->set_num_reverse_channels(play_ss->channels);
>> + pconfig = {
>> + webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /* input stream */
>> + webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /* output stream */
>> + webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /* reverse input stream */
>> + webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /* reverse output stream */
>> + };
>> + apm->Initialize(pconfig);
>>
>> if (hpf)
>> apm->high_pass_filter()->Enable(true);
>>
>> if (!mobile) {
>> - if (ec->params.drift_compensation) {
>> - apm->echo_cancellation()->set_device_sample_rate_hz(out_ss->rate);
>> - apm->echo_cancellation()->enable_drift_compensation(true);
>> - } else {
>> - apm->echo_cancellation()->enable_drift_compensation(false);
>> - }
>> -
>> + apm->echo_cancellation()->enable_drift_compensation(ec->params.drift_compensation);
>> apm->echo_cancellation()->Enable(true);
>> } else {
>> apm->echo_control_mobile()->set_routing_mode(static_cast(rm));
>> @@ -225,7 +223,7 @@ fail:
>> if (ma)
>> pa_modargs_free(ma);
>> if (apm)
>> - webrtc::AudioProcessing::Destroy(apm);
>> + delete apm;
>>
>> return false;
>> }
>> @@ -235,10 +233,13 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const uint8_t *play) {
>> webrtc::AudioFrame play_frame;
>> const pa_sample_spec *ss = &ec->params.priv.webrtc.sample_spec;
>>
>> - play_frame._audioChannel = ss->channels;
>> - play_frame._frequencyInHz = ss->rate;
>> - play_frame._payloadDataLengthInSamples = ec->params.priv.webrtc.blocksize / pa_frame_size(ss);
>> - memcpy(play_frame._payloadData, play, ec->params.priv.webrtc.blocksize);
>> + play_frame.num_channels_ = ss->channels;
>> + play_frame.sample_rate_hz_ = ss->rate;
>> + play_frame.interleaved_ = false;
>
> Using non-interleaved data looked strange, but I thought that you
> probably know what you are doing. Then I accidentally noticed that
> patch 19 is a fixup for the interleaved setting. Why is patch 19 not
> squashed into this initial patch? Are there other similar fixups? Could
> you resend with fixup patches squashed into the patches that introduce
> the fixed bugs?
The problem is that as the changes came later, squashing became
progressively harder. I've squashed a few of these. The ones that
remain are related to some blocksize calculations when channel counts
for in/out/playback are not the same. Resending that now.
-- Arun
