search

seanbright / asterisk-opus

Opus (transcoding) and VP8 (passthrough) support for Asterisk, needed for a better WebRTC integration
35 stars 14 forks source link

Patchless build. #21

Closed p120ph37 closed 7 years ago

p120ph37 commented 8 years ago

No patches against Asterisk core, so the modules can be built with just the Asterisk headers and used with existing Asterisk 13 binaries. Maybe this will be enough to get it into the major distros as an Asterisk add-on package? :-)

HansVanEijsden commented 8 years ago

Great, thanks! 😃

Maybe it's an idea to auto detect the modules path? In your commit, the path is hardcoded: +ASTMODDIR=/usr/lib64/asterisk/modules but my modules are in /usr/lib/asterisk/modules as suggested in the documentation and other tutorials.

And I also see a hardcoded gcc rule, but I like to build everything with -flto and -march=native for even more performance (-O3 -march=native -flto). What to do with that?

p120ph37 commented 8 years ago

I was trying to just get things working with a fairly simple Makefile rather than doing a whole autoconf thing. If your modules are in a different path, just override the variable on the command-line like this: make install ASTMODDIR=/usr/lib/asterisk/modules Or feel free to improve my Makefile!

HansVanEijsden commented 8 years ago

Haha that's even better. Happy with your solution, thanks again!

scgm11 commented 8 years ago

is this PR going to be merged?

traud commented 7 years ago

Aaron, thank you very much for your Pull Request. I like this idea very much, not just because of the rationales you posted, but because your approach avoids several patch-fuzz issues as well. Your solution is so simple and clever – now, why didn’t I think of that? Thanks again for your solution.

I hope you do not mind that I did not merge it here. Here, this repository only sees bug fixes (see the starting page of this repository why). I included your change in my fork, which adds new features as well. Furthermore, I hope you do not mind that I did not fork your repository and merged/linked it that way. Well, it was easier for to me to hijack the file. I changed just a bit, because Asterisk tries to be ANSI-C compatible (C90). If you go for ./configure --enable-dev-mode, you see what I mean. Beside that, I did not change much. Perhaps you find the time to double-check my inclusion.

I did not update the Read Me file, yet. I am going to do that in the next few days. As you see, the file asterisk.patch is still there. Anyway, thanks to your change, just modules can be added/replaced. As you stated, there is no need to alter the core of Asterisk anymore. I am sure this is going to avoid a lot of headaches in the future. It avoids two fuzz-level 2 already.

traud commented 7 years ago

Upps, I just noticed, I missed your updated Makefile. I am going to change that tomorrow (or you create a Pull Request against my repository/asterisk-13.7).

p120ph37 commented 7 years ago

Thanks. Glad this patch was more straightforward than my iLBC fix :-P

traud commented 7 years ago

Your iLBC patch is/was straightforward as well. Furthermore, it solved your issues. However, I am not yet able to reproduce that ptime=30 issue and the reported problems with my iLBC 20 patch. The first step is understanding the cause – for that, I have to reproduce your scenario. Therefore, please, answer the last two questions in ASTERISK-26221…