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

[Tanu Kaskinen]

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?

-- 
Tanu