Added encoder complexity and codecs.conf parsing

[lminiero]

As promised, here's a simple PR that adds a call to the OPUS_SET_COMPLEXITY control in order to set up the complexity of the encoder when creating it. The default complexity value used in this patch, as specified in a new #define in opus.h, is 4, which gives a good quality/cpu tradeoff, but you can override it in codecs.conf.

This PR, in fact, also adds codecs.conf parsing to the module. At load/reload it looks for the [opus-open-source] context, and can override some of the defines, namely complexity, default bitrate, cbr/vbr, FEC and DTX. Other settings can be added later on, if needed. An example of a working context is as follows:

[opus-open-source]
complexity => 4
maxaveragebitrate => 510000
cbr => false
useinbandfec => true
usedtx => false

The PR doesn't modify the stock codecs.conf, as that would likely require an ad-hoc patch to apply rather than a brand new file to copy over the existing one, in order to avoid loosing new settings that other codecs may add in future versions. I guess that simply clarifying in the README you can configure some settings will probably be enough.

[traud]

The Asterisk team seems to have different defaults in their opus.h. I know, you prefer 4 from your practical experience. Anyway, is it possible to align those values somehow – either by using the same as the Asterisk team or by changing their value?

You created a new section called ‘opus-open-source’ in codecs.conf. Is that required – really, I have not looked into it yet – cannot we reuse their existing section ‘opus’?

[lminiero]

I chose a different name in case one has both plugins installed, so that you could configure them differently: not sure that would even work, actually, but that was the rationale behind it. Of course that can be changed.

[traud]

Puh. What would be the (potential) benefit of having two modules installed for the same audio format at the same time. I have problems to envision such a benefit, therefore that rationale did not come to mind before.

[lminiero]

I'm not saying it makes sense, I'm saying I wanted to leave doors open to the possibility and that's why I did it like that 😉

[traud]

Please, go the other way. Align with the Digium solution as much as possible (complexity, section name). If there is no known rationale, making something different is just more complex. If you like, you can add a comment about your experience (4 is a good default for a scenario of…; if you need a separate section name, please, create a Pull Request via Github…).

Or stated differently: Any difference needs a rationale in my world, because users do not except differences but uniform/known behavior. I love good defaults. However, if you want to change the default, convince the Asterisk team.

[lminiero]

Apologies for the awfully late reply, but I've been pretty swamped. I'll try and adapt the patch as you suggested as soon as I have some time to do that.

[mohit-dhiman]

Hi,
I tried this patch and the problem that i am facing is that none of the configurable parameters when changed gets reflected at the SDP of INVITE or 200 OK packets sent by the Asterisk.
Can anybody comment on it if i am doing something wrong or is this expected because binary module of codec_opus (Distributed by Sangoma) sets the parameters in SDP.

[traud]

The question is: How do you configure the module and do you have installed just this module (and not the one from Sangoma Digium as well). Because this Pull Request (PR) here, was not merged yet, you still have to change the source code to configure this module. Did that answer your question?

[mohit-dhiman]

I have installed this module (not the one provided by Digium) and i also did changes to the code according to this PR, but still the SDP didn't get updated.
I have also added some logs to verify the same thing. I think encoder is getting the changed parameters but not the resource module that changes the SDP.