zephyrproject-rtos / liblc3codec

LC3 codec implementation
63 stars 37 forks source link

Zephyr module definition make file and c wrapper #4

Closed Casper-Bonde-Bose closed 2 years ago

Casper-Bonde-Bose commented 2 years ago

Make file that builds the module as a lib using the zephyr make system. It enforces use of C++11 as that is required for the library. As the library is c++ a wrapper is needed to be able to call it from the Zephyr C applications. This change includes a wrapper for the common usage of the lib in a Zephyr context but can be extended as needed. The wrappers do not have a copy of the API documentation but refers to the c++ documentation with a few comments relevant for the usage. This commit is tested using the posix build of the LE-Audio unicast test applications with encoded LC3 data transferred over a Bleutooth CIS link. As a change ID is needed in this repo to be able to build the Zephyr code this change needs to be merged before the zephyr change can build.

Signed-off-by: Casper Bonde casper_bonde@bose.com

Casper-Bonde-Bose commented 2 years ago

Hi @aescolar, I see you uploaded the initial source code. I have created this PR to enable usage from Zephyr, but I do not have permissions to assign reviewers and no one is assigned automatically, hence I'm not sure what to do next? Thanks Casper

aescolar commented 2 years ago

CC @carlescufi @asbjornsabo @jhedberg @wopu-ot @thoh-ot ^^

Casper-Bonde-Bose commented 2 years ago

Partial solution for: https://github.com/zephyrproject-rtos/zephyr/issues/41228

Thalley commented 2 years ago

@Casper-Bonde-Bose Btw, I am still not able to resolve my own comments, so please resolve any that you feel have been resolved :)

MaureenHelm commented 2 years ago

Hi @aescolar, I see you uploaded the initial source code. I have created this PR to enable usage from Zephyr, but I do not have permissions to assign reviewers and no one is assigned automatically, hence I'm not sure what to do next? Thanks Casper

I added the bluetooth team to this repo, so you should be able to add reviewers now.

Casper-Bonde-Bose commented 2 years ago

Could we merge this change?

Casper-Bonde-Bose commented 2 years ago

Could you please move all those files to the main tree, in modules/liblc3codec ? Given that all of this is Zephyr-specific code I'd rather see it there

@carlescufi Could you please elaborate on what you mean here? I tried to follow the guidance for modules - that they should be as close as possible to the external module from which they origin. And looking at other modules all zephyr specific additions were added to a zephyr folder within each module. So what exactly do you want moved and to where? If you want me to move it to a main branch in the liblc3codec project, then I don't think I have permissions to create branches (I have not tried lately though). If you want me to move files from the zephyr folder to the root of the project, I would like to know why this is preferred - in the root you will mix files with what ever comes in the external project, which could be a mess and would require me to add a zephyr pre-fix to all files to be sure they do not clash with what ever would be added to the external project in the future.

Casper-Bonde-Bose commented 2 years ago

@aescolar @Thalley @carlescufi As requested via Discord I updated to latest codec from Android - the C-version. I do not know if we want to keep this one? The new one is here: https://github.com/zephyrproject-rtos/liblc3codec/pull/6 I cannot assign reviewers. @MaureenHelm I think the Bluetooth team was split into two and there is a new team called something with LE Audio.

aescolar commented 2 years ago

do you want this one closed?

MaureenHelm commented 2 years ago

I cannot assign reviewers. @MaureenHelm I think the Bluetooth team was split into two and there is a new team called something with LE Audio.

Not that I can tell. @carlescufi is this something you were going to do?

carlescufi commented 2 years ago

I cannot assign reviewers. @MaureenHelm I think the Bluetooth team was split into two and there is a new team called something with LE Audio.

Not that I can tell. @carlescufi is this something you were going to do?

We've solved this offline and it' now all clear.

carlescufi commented 2 years ago

Closing in favor of #8