Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LC3plus support #479

Closed
anonymix007 opened this issue Jul 29, 2021 · 20 comments · Fixed by #529
Closed

LC3plus support #479

anonymix007 opened this issue Jul 29, 2021 · 20 comments · Fixed by #529

Comments

@anonymix007
Copy link
Contributor

anonymix007 commented Jul 29, 2021

LC3plus seems to be a pretty good codec for high resolution audio over Bluetooth (non plus version should be very similar and this may help in preparation for LE Audio).
This codec has some advantages over LDAC: both decoding and encoding is supported in open-source software and (theoretically) it could give higher bitrate.
It may also happen that it will be added into Android 12 (but probably not).

@arkq
Copy link
Owner

arkq commented Jul 29, 2021

Adding new A2DP codec without sink/source devices is pointless. In order to use better audio quality (doubtful that it can be achieved, more likely better energy efficiency) one would need e.g. headphones with such codec support. Are there any headphones on the market which supports LC3plus as a codec for BT 4.0 audio? As for LE audio support in BlueZ, I think that we will have to wait rather longer (there are missing some crucial components in BlueZ).

@anonymix007
Copy link
Contributor Author

There is a high possibility that headphones with LC3plus will appear on market (not sure when, prototype is expected to exist before the end of summer).
High resolution mode sounds promising in terms of sound quality (and I was not able to tell the difference between it (encoded and then decoded) and lossless).
This may also have (at least) some point for using this for transferring audio between Linux-enabled devices (any SBC as media center and PC).

@arkq
Copy link
Owner

arkq commented Jul 29, 2021

OK, do you have any spec regarding A2DP configuration blob and data transport headers (if any)? Without these information adding support for new codec will work only between two bluealsa-enabled devices. If we want to target for new headphones, we need some specification how to implement it.

@anonymix007
Copy link
Contributor Author

I've contacted Fraunhofer IIS about that not long time ago, as for now they're settling bureaucratic things.
The problem is that it may happen that I won't be able to share documentation publicly, so I've to write this part on my own (for which I'll certainly need some assistance).

@arkq
Copy link
Owner

arkq commented Jul 29, 2021

OK, signing NDA from my side is not a problem as long as the final implementation could be publicly released (it does not have to be released right away, but e.g. shortly after first product with such codec is on the market). For example, similar case is with Sony's LDAC decoder - bluealsa has support for it but library is not publicly available.

@anonymix007
Copy link
Contributor Author

As for NDA, developers will probably know better.
Actually, LDAC decoder is publicly available, there's also WIP reverse-engineered library version which won't compile at the moment but pretty close to that.

@arkq
Copy link
Owner

arkq commented Jul 30, 2021

Actually, LDAC decoder is publicly available

Nice, I will compare it with original decoder.

WIP reverse-engineered library version which won't compile

There is rather a long way to go. A lot is missing :)

Beside decoder/encoder availability, there is also a licensing problem. Sony requires certification if one wants to use LDAC technology. https://android.googlesource.com/platform/external/libldac/+/refs/heads/master/NOTICE

@anonymix007
Copy link
Contributor Author

There is description of RTP payload format for the LC3plus codec in the official public document. Is this enough or something other is required too?

@arkq
Copy link
Owner

arkq commented Jul 31, 2021 via email

@anonymix007
Copy link
Contributor Author

anonymix007 commented Jul 31, 2021

It looks like that for this spec one would need to contact Fraunhofer IIS. I had already done that, you may do it too

@arkq
Copy link
Owner

arkq commented Aug 10, 2021

I've sent a message to Fraunhofer via web page form and I'm waiting for a reply :)

@arkq
Copy link
Owner

arkq commented Jan 29, 2022

@anonymix007 If you are still interested in LC3plus support, the PR is ready :) I will have to perform few more test before merging it into the master, though.

@anonymix007
Copy link
Contributor Author

@arkq Thanks, that's great.
BTW which library are you using? ETSI LC3plus software compiles into only standalone executable for encoding/decoding, not into shared library.

@arkq
Copy link
Owner

arkq commented Jan 30, 2022

BTW which library are you using? ETSI LC3plus software compiles into only standalone executable for encoding/decoding, not into shared library.

Yes, I'm using the lib available via ETSI. And yes, provided Makefile does not produce library. One will have to modify the Makefile or use other build system for source files. However, simple modification like below will suffice:

From de4e3c18ba73419c57481442aeefda29e94d3a04 Mon Sep 17 00:00:00 2001
From: Arkadiusz Bokowy <[email protected]>
Date: Thu, 9 Dec 2021 20:50:44 +0100
Subject: [PATCH] Makefile targets for building shared libraries

---
 src/fixed_point/makefile    | 20 ++++++++++++++------
 src/floating_point/makefile | 15 ++++++++++++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/fixed_point/makefile b/src/fixed_point/makefile
index 01d5725..33f99ed 100644
--- a/src/fixed_point/makefile
+++ b/src/fixed_point/makefile
@@ -15,6 +15,8 @@ LINK   = $(CC)
 
 # Binary Name
 NAME_LC3   = LC3plus
+# Shared Library Name
+LIB_LC3    = libLC3plus.so
 
 # Default tool settings
 RM        = rm -f
@@ -25,12 +27,11 @@ QUIET_LINK= @echo '   ' Linking $@;
 QUIET     = @
 endif
 
-CFLAGS += -std=c99
-
 # C compiler flags
 # Preprocessor(-I/-D) / Compiler / Linker flags
 CPPFLAGS += -Ibasic_op -DSUBSET_$(SUBSET)
-CFLAGS   += -pedantic -Wcast-qual -Wall -W -Wextra -Wno-long-long     \
+CFLAGS   += -std=c99 -fPIC                                            \
+            -pedantic -Wcast-qual -Wall -W -Wextra -Wno-long-long     \
             -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes  \
             -Werror-implicit-function-declaration
 
@@ -84,11 +85,14 @@ OBJS_CCC := $(filter-out $(EXCL_CCC), $(OBJS_CCC_UNF))
 
 OBJS := $(addprefix $(BUILD)/, $(SRCS:.c=.o))
 
+LIBSRCS := $(filter-out $(DIR)/ccConvert.c $(DIR)/codec_exe.c, $(SRCS))
+LIBOBJS := $(addprefix $(BUILD)/, $(LIBSRCS:.c=.o))
+
 ###############################################################################
 
 .PHONY: all clean help force
 
-all: $(NAME_LC3)
+all: $(NAME_LC3) $(LIB_LC3) ccConvert
 
 help:
 	@echo 'Targets:'
@@ -108,13 +112,17 @@ help:
 $(NAME_LC3): $(OBJS)
 	@echo 'Linking' $@
 	$(QUIET) $(LINK)  $(OBJS) -o $@ $(LDFLAGS)
-    
+
+$(LIB_LC3): $(LIBOBJS)
+	@echo 'Linking' $@
+	$(QUIET) $(LINK) --shared $(OBJS) -o $@ $(LDFLAGS)
+
 ccConvert: $(BUILD)/ccConvert.o $(OBJS_CCC)
 	@echo 'Linking' $@
 	$(QUIET) $(LINK) $? -o $@ $(LDFLAGS)
 
 clean:
-	$(QUIET) rm -rf $(NAME_LC3) $(BUILD) ccConvert
+	$(QUIET) rm -rf $(NAME_LC3) $(LIB_LC3) $(BUILD) ccConvert
 
 $(BUILD)/%.o : %.c $(BUILD)/cc_flags
 	@echo 'Compiling' $<
diff --git a/src/floating_point/makefile b/src/floating_point/makefile
index 8818c16..898d7c0 100644
--- a/src/floating_point/makefile
+++ b/src/floating_point/makefile
@@ -15,13 +15,15 @@ LINK   = $(CC)
 
 # Binary Name
 NAME_LC3   = LC3plus
+# Shared Library Name
+LIB_LC3    = libLC3plus.so
 
 # Default tool settings
 CC        = gcc
 RM        = rm -f
 
 # Preprocessor(-I/-D) / Compiler / Linker flags
-CFLAGS   += -std=c99 -O$(OPTIM)                                                            \
+CFLAGS   += -std=c99 -fPIC -O$(OPTIM)                                                      \
             -Wall -pedantic -Wcast-qual -Wno-long-long -Wpointer-arith -Wstrict-prototypes \
             -Wmissing-prototypes -Werror-implicit-function-declaration -Wunused-parameter  \
             -Wunused-macros -Wunused-local-typedefs -g
@@ -91,11 +93,14 @@ POSTCOMPILE = mv -f $(BUILD)/$*.Td $(BUILD)/$*.d && touch $@
 SRCS := $(notdir $(foreach DIR, $(VPATH), $(wildcard $(DIR)/*.c)))
 OBJS := $(addprefix $(BUILD)/, $(SRCS:.c=.o))
 
+LIBSRCS := $(filter-out $(DIR)/codec_exe.c, $(SRCS))
+LIBOBJS := $(addprefix $(BUILD)/, $(LIBSRCS:.c=.o))
+
 .PHONY: all clean help force
 
 .PRECIOUS: $(BUILD)/%.d
 
-all: $(NAME_LC3)
+all: $(NAME_LC3) $(LIB_LC3)
 
 help:
 	@echo 'Syntax: make [OPTION=VALUE ...]'
@@ -111,8 +116,12 @@ $(NAME_LC3): $(OBJS)
 	@echo 'Linking' $@
 	$(QUIET) $(LINK)  $(OBJS) -o $@ $(LDFLAGS)
 
+$(LIB_LC3): $(LIBOBJS)
+	@echo 'Linking' $@
+	$(QUIET) $(LINK) --shared $(OBJS) -o $@ $(LDFLAGS)
+
 clean:
-	$(QUIET) rm -rf $(NAME_LC3) $(BUILD)
+	$(QUIET) rm -rf $(NAME_LC3) $(LIB_LC3) $(BUILD)
 
 $(BUILD)/%.o : %.c $(BUILD)/cc_flags
 	@echo 'Compiling' $<
-- 
2.34.1

And there are other technical issues with this library... Unfortunately, a new version might no be available soon (I hope that a new version will be released this year...). Also, I'm not sure whether the build system will be changed or at least modified in a way shown in the patch above. I've been talking with Fraunhofer about that, and my impression is that "the simplest" way of adapting this library by the opensource community is to create e.g. github repository and maintain a build system separately from Fraunhofer (the same way the https://github.com/mstorsjo/fdk-aac is maintained by Martin Storsjö).

(If anyone from Fraunhofer is reading this and is willing to have a public conversation, please correct me if I'm wrong abut the maintenance of LC3plus library in a way compatible with Unix and opensource in general.)

arkq added a commit that referenced this issue Jan 30, 2022
arkq added a commit that referenced this issue Feb 1, 2022
@arkq arkq closed this as completed in 75a3d87 Feb 1, 2022
@anonymix007
Copy link
Contributor Author

@arkq
I'm not totally sure, but isn't HR mode supposed to be turned on even if 48 kHz (not only 96 kHz) sample rate is used?

@arkq
Copy link
Owner

arkq commented Feb 1, 2022

Text from lc3.h file for lc3plus_enc_set_hrmode function:

Set the high resolution mode for LC3 encoder. This mode is mandatory for 96 kHz input and can also be used for 48 kHz input. Encoder and decoder must have the same high resolution mode active.

Of course I might add command line option for enabling it even for lover sample rate. Current lc3plus support is a "bare" minimal in order to fulfill all spec and codec requirements.

EDIT:
The problem with this setting is that it has to be enabled for encoder and decoder, but this information is not transmitted via side channel (A2DP configuration blob). So to be honest I'm not sure. I will ask Fraunhofer about that. Thanks for pointing that out.

@arkq
Copy link
Owner

arkq commented Feb 1, 2022

After reading A2DP spec one more time, I think you are right :D For both sampling frequencies the HR mode has to be enabled. I will fix that.

@arkq
Copy link
Owner

arkq commented Jul 3, 2023

Since libLC3plus v1.4.1 the patch from the #479 (comment) is no longer required (it was merged into the upstream). In order to build the library one will have to make the libLC3plus.so target as follows: make libLC3plus.so. Also, since commit d584239 bluez-alsa requires libLC3plus >= v1.4.1.

EDIT:
Since commit 0eefbf1 bluez-alsa searches for lc3plus.h instead of lc3.h. The latter one clashes with the header file for LC3 codec. However, the upstream version of LC3plus codec is shipped with lc3.h, so one will have to rename it when compiling bluez-alsa. Alternatively, one can use my mirror of LC3plus: https://github.com/arkq/LC3plus

@anonymix007
Copy link
Contributor Author

liblc3 now has support for LC3plus (including HR), so maybe it's worth fully switching to it?

@arkq
Copy link
Owner

arkq commented Jan 9, 2024

Yes, it might be a good idea to add support for Google's implementation of LC3plus. However, I'd like to keep support for reference implementation (which can be downloaded from etsi.org) too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants