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

[Arun Raghavan]

On 16 December 2015 at 09:52, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Wed, 2015-12-16 at 09:48 +0530, Arun Raghavan wrote:
>> On 16 December 2015 at 09:38, Tanu Kaskinen <tanuk at iki.fi> wrote:
>> > (I'm just glancing through, this is not a proper review.)
>> >
>> > 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])],
>> >
>> > I think it would be better to use >= 0.2 (or whatever is the
>> > minimum
>> > required version).
>>
>> Sure, I can do that. Was easier to test this way before doing a 0.2
>> release of webrtc-audio-processing.
>>
>> > >      [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
>> >
>> > This seems like material for a separate patch.
>>
>> This is needed for the code to compile at all, is why I included it
>> here.
>
> Ok, makes sense. It would be good to note the reason in the commit
> message, though. Does the requirement for c++11 come from the library
> or from the code changes in this patch?

The library IIRC (will double-check and update the commit message).

-- Arun