-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add support for LHDC v2 A2DP source and sink #742
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #742 +/- ##
==========================================
+ Coverage 70.45% 70.46% +0.01%
==========================================
Files 96 96
Lines 16024 16024
Branches 2516 2516
==========================================
+ Hits 11289 11291 +2
+ Misses 4616 4614 -2
Partials 119 119 ☔ View full report in Codecov by Sentry. |
@anonymix007 I'm trying to add support for LHDC v2 using your library, but it seems that something is not right. As I've mentioned earlier, I have a retail device which is able to stream LHDC v2 codec (Huawei Mate 20 Pro). Unfortunately, the
It seems that the decoder is not able to decode most of the frames... Used A2DP configuration: $ a2dpconf lhdc-v2:3a050000324c140001
LHDC v2 <hex:3a050000324c140001> {
vendor-id:32 = 0x0000053a [Savitech Corp.,]
vendor-codec-id:16 = 0x4c32
<reserved>:2
bit-depth:2 = 24
sample-rate:4 = 48000
low-latency:1 = false
max-bitrate:3 = 900
version:4 = 0
<reserved>:4
ch-split-mode:4 = None
} However, when stream is created with Encoded and decoded sine with patch as follows: diff --git a/src/a2dp-lhdc.c b/src/a2dp-lhdc.c
index f6d50e7..08fc4a2 100644
--- a/src/a2dp-lhdc.c
+++ b/src/a2dp-lhdc.c
@@ -303,7 +303,11 @@ void *a2dp_lhdc_enc_thread(struct ba_transport_pcm *t_pcm) {
if (codec_id == A2DP_CODEC_VENDOR_ID(LHDC_V2_VENDOR_ID, LHDC_V2_CODEC_ID)) {
- if ((rv = lhdcBT_encode(handle, input, bt.tail)) < 0) {
+ int32_t xxx[2048] = { 0 };
+ for (size_t i = 0; i < lhdc_ch_samples; i++)
+ xxx[i] = input[i*2];
+ if ((rv = lhdcBT_encode(handle, xxx, bt.tail)) < 0) {
error("LHDC encoding error: %d", rv);
break;
}
Left channel is garbage, and the encoded channel is decoded as a right one. Also, it seems that the Do you have any idea what might be wrong with v2 here? In next few days I'll try to get some Android phone with LHDC v3 to test the v3 decoder. |
PCM data ends up in this function:
I'll take a look at the dump a bit later.
This could be a bug in my implementation or normal behavior, I'm not sure. Is it happening for just a few first frames or is it consistent?
AFAIU it won't work because sink devices do not do anything to defragment payloads afterwards. |
These don't look like correct LHDC frames to me.
The actual LHDC frames are:
So first packet has one frame and a part of the second one and second one contains the rest.
Second is
Note that total size is 560+628=1188 which is actually the combined length of payloads for the first 2 packets.
It isn't s32le. Depending on the bit depth parameter, it will be either s24le (packed/3 bytes) or s16le interleaved stereo.
Actually, now that I'm looking at the AOSP patches for V2 support, it seems that there indeed is some sort of fragmentation. However, I don't think my liblhdcBT_dec supports |
This mask is a copy of struct rtp_lhdc_media_header {
uint8_t fragmented:1;
uint8_t first_fragment:1;
uint8_t last_fragment:1;
uint8_t frame_count:3;
uint8_t latency:2;
uint8_t seq_number;
} I was able to add defragmentation logic into the decoder, unfortunately, the decoded audio is still far from original :D So, I have another question, is the
And in case of 2 frames I can see:
But the I'll try to feed frame by frame (maybe later today) and see what will happen :) |
I think it should be. You may try by converting packets to the format that my lhdcdec.c tool understands, that is, each packet is prepended by struct hdr {
uint16_t len;
uint16_t frames;
};
This is bad, correct frames should not trigger this.
How much samples does it decode? Maybe the issue is with the second frame. |
Splitting LHDC packets with 2 frames into two packets yields the same result. But maybe I'm doing something wrong (something is not 100% right). Steps are as follows:
So, I guest that the same algorithm is in your decoding library, so it doesn't matter whether I pass 2 frames in a single packet, or 2 packets with a single frame. But there is one disclaimer, I'm not sure whether the Huawei implementation is correct :D I do not have a retail sink device which could consume LHDC v2 from my phone.... There was a time when Android's AAC implementation was incorrect.... |
This is wrong. My parser reports the following:
And the second one is:
More specifically, "2 bytes after sync" part is wrong. If you look at my parser code (which I attached in the original PR), you'll see for (size_t i = 0; i < packet_size / sizeof(uint32_t); i++) {
((uint32_t*)packet)[i] = __builtin_bswap32(((uint32_t*)packet)[i]);
} I have no idea why they decided to byte-swap the encoded frames, maybe it has something to do with the circular buffer implementation for some platforms. |
Oooo, I see. I've updated the code so now the len is
Do you have any idea what that might be? Anyway decoder is not able to decode the second frame in the packet. I've fed the decoder with media header + frame for frame1 and frame2 without this residual data (I've check adding the residual data but the effect is the same). I've been checking today Redmi Note 13, OPPO Reno 12 Pro and OPPO A40 with BlueALSA but for some reasons neither of these phones was not able to select LHDC codec. In developer tools phones report:
but selecting these codecs does not send AVDTP SetConfiguration requests... Everything works fine for AAC, aptX, aptX HD, LDAC and SBC, though. I'll try to do some tests with some other phones later next week. Also I'll try to request codec lists from tested phones, so I will see capabilities exposed by tested phones (I do not have such data now). |
All BBK phones limit LHDC to their own headphones. But you may try to bypass that by renaming sink to "OnePlus".
No, these do not look like anything meaningful. Oh, and AFAIR mask should be 0x1F for v2/v3, but 0x0F for v4 because of the "extension" flag. This wouldn't matter for most frames though. Can you try "patching" the length so that it will include these bytes? |
Xiaomi phones don't enable LHDC by default for third-party headsets, but you can manually enable it after Bluetooth pairing. |
This PR is a follow-up for #672