On 10. 04. 2018 14:26, Tanu Kaskinen wrote:
On Mon, 2018-04-09 at 11:56 +0200, Tomaž Šolc wrote:
In general I think there's a question of how heavy a wrapper the
"module-echo-cancellation" is supposed to be around the webrtc code. The
upstream is very actively reworking large parts of the code. Most
likely closely following the changes and ensuring backwards
compatibility in PulseAudio, even at the level of module arguments,
isn't feasible in the long term.

When there's a pressing need to break compatibility, then that can be
done, but not otherwise.

I see. I restored back the "trace" argument.

The lack of documentation is a valid point. Generally I'm of the
opinion is that if some API isn't documented, it shouldn't be used. In
module-echo-cancel's case that would mean most of the webrtc options
shouldn't be used, which of course isn't a good...

Unfortunately the whole webrtc part of module-echo-cancel is an exercise in using an internal, undocumented API that is subject to change at any time. See e.g. discussion here. It's from 2012 but AFAIK this situation has not changed:

https://groups.google.com/forum/#!topic/discuss-webrtc/pyxD1_BeJvs

Still, we find it useful, since it seems to be the best open source echo canceller available at the moment.

I removed this default because the new webrtc configuration class seems
to define some sensible defaults itself. Individual components no longer
need to be explicitly enabled/disabled like before.

I think upstream has a much better knowledge of which defaults make
sense. For example, high pass filter is disabled by default in the
current upstream (while DEFAULT_HIGH_PASS_FILTER was set to true).

Shouldn't we then remove all the other defaults as well?

The upstream code is in the middle of migration between two configuration systems:

Some settings use the new webrtc::AudioProcessing::Config class. That class includes some defaults that have been defined upstream. I made module-echo-cancel respect those defaults (high_pass_filter, residual_echo_detector).

Other settings still use the old webrtc::Config class. That class has no defaults. Hence I left in place the old DEFAULT_... macros.

Best regards
Tomaž
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to