asterisk / asterisk

The official Asterisk Project repository.
https://www.asterisk.org
Other
1.97k stars 924 forks source link

format_aac: Advanced Audio Coding (mp4) #693

Closed viktike closed 3 weeks ago

viktike commented 1 month ago

MPEG4 Advanced Audio Coding implementation using two external libraries: libfaac (encoder) and libfaad2 (decoder).

Fixes: #692

UserNote: 8khz, 16khz, 32khz and 48khz sampling rates are available with the proper file extensions: aac, aac8, aac16 and aac48 suffixes.

viktike commented 1 month ago

cherry-pick-to: 21 cherry-pick-to: 20 cherry-pick-to: 18

viktike commented 1 month ago

I think you can move the module into the formats directory.

Moved to formats/

jcolp commented 1 month ago

What is the license of the libraries being linked to?

viktike commented 1 month ago

What is the license of the libraries being linked to?

The libraries are provided by the operating system, not included in Asterisk:

gtjoseph commented 1 month ago

Just a note... Those libraries aren't packaged in any RedHat based distribution so the current CI isn't even going to compile them.

viktike commented 1 month ago

Just a note... Those libraries aren't packaged in any RedHat based distribution so the current CI isn't even going to compile them.

https://rpmfusion.org/ have them, and they are called faac-devel and faad2-devel.

gtjoseph commented 1 month ago

Just a note... Those libraries aren't packaged in any RedHat based distribution so the current CI isn't even going to compile them.

https://rpmfusion.org/ have them, and they are called faac-devel and faad2-devel.

Yes, I know and I have them locally. I'm just saying the CI stack doesn't currently include those repos.

InterLinked1 commented 1 month ago

Just a note... Those libraries aren't packaged in any RedHat based distribution so the current CI isn't even going to compile them.

https://rpmfusion.org/ have them, and they are called faac-devel and faad2-devel.

Yes, I know and I have them locally. I'm just saying the CI stack doesn't currently include those repos.

What if the Fedora packages were added to install_prereq? Does the CI run that script?

viktike commented 1 month ago

Just a note... Those libraries aren't packaged in any RedHat based distribution so the current CI isn't even going to compile them.

https://rpmfusion.org/ have them, and they are called faac-devel and faad2-devel.

Yes, I know and I have them locally. I'm just saying the CI stack doesn't currently include those repos.

What if the Fedora packages were added to install_prereq? Does the CI run that script?

The repository (rpmfusion) still needed to be added. (add to yum.repos.d, import the gpg private key).

gtjoseph commented 1 month ago

I chatted with @jcolp and we'll add the rpmfusion-free repo to the CI images and install the two packages. Might be Wednesday before I can get to it though.

viktike commented 1 month ago

I chatted with @jcolp and we'll add the rpmfusion-free repo to the CI images and install the two packages. Might be Wednesday before I can get to it though.

faad2 is in free: https://ftp-stud.hs-esslingen.de/pub/Mirrors/rpmfusion.org/free/fedora/development/rawhide/Everything/x86_64/os/repoview/faad2-devel.html faac is in non-free: https://ftp.icm.edu.pl/pub/Linux/dist/rpmfusion/nonfree/fedora/development/rawhide/Everything/x86_64/os/repoview/faac-devel.html

According to the RepoView

viktike commented 1 month ago

Applied @seanbright 's recommended changes from #704

gtjoseph commented 1 month ago

I chatted with @jcolp and we'll add the rpmfusion-free repo to the CI images and install the two packages. Might be Wednesday before I can get to it though.

faad2 is in free: https://ftp-stud.hs-esslingen.de/pub/Mirrors/rpmfusion.org/free/fedora/development/rawhide/Everything/x86_64/os/repoview/faad2-devel.html faac is in non-free: https://ftp.icm.edu.pl/pub/Linux/dist/rpmfusion/nonfree/fedora/development/rawhide/Everything/x86_64/os/repoview/faac-devel.html

According to the RepoView

Yeah, we need to check the implications of faac being "nonfree"

jcolp commented 1 month ago

Yes, before anything new is accepted into the tree that embeds outside code or links against a library I have to thoroughly review licensing and patent concerns. When licenses like LGPL are involved, I have to remember the finer points of things as over all adding new things that require other libraries is rare. There is also the concern that at the bottom of the README for faac that it talks about potential patent payment.

viktike commented 1 month ago

Yes, before anything new is accepted into the tree that embeds outside code or links against a library I have to thoroughly review licensing and patent concerns. When licenses like LGPL are involved, I have to remember the finer points of things as over all adding new things that require other libraries is rare. There is also the concern that at the bottom of the README for faac that it talks about potential patent payment.

If it's not viable to merge these modules into the tree, I can do a repository under my github user with patches for Asterisk.

jcolp commented 3 weeks ago

I have finished refreshing my memory on licensing. The project will not be able to accept this module due to its dependency requirements, which are licensing incompatible and also have patent concerns. I am closing out this pull request accordingly.