[pulseaudio-discuss] [PATCH v2 01/25] echo-cancel: Update webrtc-audio-processing usage to new API

[Arun Raghavan]

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