Skip to content

Add support for AMR-WB codec. - #58

Merged
dennwc merged 6 commits into
mainfrom
codec-amrwb-oss
May 20, 2026
Merged

Add support for AMR-WB codec.#58
dennwc merged 6 commits into
mainfrom
codec-amrwb-oss

Conversation

@dennwc

@dennwc dennwc commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Use opencore-amr for decoding and vo-amrwbenc for encoding, instead of code from 3GPP spec which has unclear licensing.

Based on #57.

Use opencore-amr for decoding and vo-amrwbenc for encoding.
@dennwc
dennwc force-pushed the codec-amrwb-oss branch from 2b72447 to a9bb067 Compare May 18, 2026 17:21
@biglittlebigben

Copy link
Copy Markdown
Contributor

Would it to separate the actual C code from the CGO implementation in each directory? This would make more clear what code we copied vs wrote.

It's also not realistic to review all this C code. Not sure what this means in practice if we commit it in our own repo... Maybe we should ask for guidance about best practices for such cases? Would a git submodule make any sense? Or am I overthinking this...

@dennwc

dennwc commented May 19, 2026

Copy link
Copy Markdown
Collaborator Author

@biglittlebigben splitting C and Go code into directories will break cgo compilation/caching. Unfortunately cgo is not flexible enough in this regard. All C and associated Go files must be co-located in the same directory.

This is why I intentionally flattened all required files from these libraries, so that cgo can build them as a single package, without any external dependencies. Otherwise it will require extra headers/libraries on the host, which we'd like to avoid.

I could split this code into a separate repo, if needed. That would work.

Long-term the plan is to replace it with a pure Go implementation, similar to how G722 was translated.

@biglittlebigben biglittlebigben left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how importing a bunch of outside code in a repo we otherwise own may be flagged in an audit, but absent a compliance person to consult with, I do not have better option to confidently suggest...

@dennwc
dennwc force-pushed the codec-amrwb-oss branch from ea24da6 to fd37668 Compare May 20, 2026 13:25
@dennwc

dennwc commented May 20, 2026

Copy link
Copy Markdown
Collaborator Author

I moved the C source into a separate project. This way we could quickly swap it for something else.

@dennwc
dennwc merged commit 88258bf into main May 20, 2026
2 checks passed
@dennwc
dennwc deleted the codec-amrwb-oss branch May 20, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants