shermp / Pico-ASHA

ASHA on a Pico
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

Current status and feedback #1

Open shermp opened 2 months ago

shermp commented 2 months ago

At long last, I finally have something that kind of, sort of, works.

Current status with my Oticon More 2's:

I know I'm not yet doing everything right, especially regards to status updates. I'm also not yet doing proper multi-core sync with shared variables.

@thewierdnut , @barolo tagging you in case you're interested in providing any feedback, assistance, or testing. It's all welcome. This has been my first ever microcontroller project (as well as bluetooth and USB), and sometimes I feel I have bitten off more than I could chew.

barolo commented 2 months ago

I don't have anything that I could test it with, the only berries I own are 4Bs. It's crazy that we went from 0 projects for years to 3 suddenly popping up :D

thewierdnut commented 2 months ago

https://github.com/shermp/Pico-ASHA/blob/761b35a4182d94dfb973a25a2fd84b4a99e0c1f3/src/asha_audio.c#L44

I wouldn't bother computing mono audio unless you know you only have one device.

A personal nit you are free to ignore: Don't use bit-shifts when you mean divide. This is an averaging operation, so its easier to understand the code with a divide, and the compiler is smart enough to use a bitshift anyways.

https://github.com/shermp/Pico-ASHA/blob/761b35a4182d94dfb973a25a2fd84b4a99e0c1f3/src/asha_bt.cpp#L257-L263

For ACPStatus::conn_param_updated, there is an undocumented third parameter, a uint8_t that indicates the negotiated connection interval. See the android source code

https://github.com/shermp/Pico-ASHA/blob/761b35a4182d94dfb973a25a2fd84b4a99e0c1f3/src/asha_bt.cpp#L944

The last parameter for an ACP start isn't whether or not the other device is connected, but whether or not it is successfully streaming. The android code validates that the other side exists and that they have responded to an ACP start already. I personally don't really know what the value of this command can possibly be. The hearing devices are already supposed to be in communication with each other, and relying on the central to indicate the status of the other hearing device would be prone to race conditions.

https://github.com/shermp/Pico-ASHA/blob/761b35a4182d94dfb973a25a2fd84b4a99e0c1f3/src/asha_bt.cpp#L507

I don't exactly know the purpose of this value, but android sets it to 10

https://github.com/shermp/Pico-ASHA/blob/761b35a4182d94dfb973a25a2fd84b4a99e0c1f3/src/usb_audio.c#L378

I would defer encoding audio until the last moment before you send it. g722 is not stateless, and if you have to drop a packet, it can cause audible artifacts in the stream.

thewierdnut commented 2 months ago

I'll also note that you haven't done anything to manage stream pacing. I've had difficulty getting the pacing right on linux because I don't have direct access to the bt stack. The android source code will preemptively drop packets if:

  1. Either side ran out of credits
  2. The number of credits from the left and right sides differ by more than four.
shermp commented 2 months ago

@barolo no worries. If you ever do decide to pick up a Pico W or two, feel free to test. They are super cheap and readily available (at least where I live).

@thewierdnut thank you so much for your feedback!

I wouldn't bother computing mono audio unless you know you only have one device.

A personal nit you are free to ignore: Don't use bit-shifts when you mean divide. This is an averaging operation, so its easier to understand the code with a divide, and the compiler is smart enough to use a bitshift anyways.

Yeah, I'm tossing up about the mono stuff. That bit shift math was left over from the UAC2 example that I adapted.

For ACPStatus::conn_param_updated, there is an undocumented third parameter, a uint8_t that indicates the negotiated connection interval. See the android source code

Thanks for pointing that out. I haven't actually been looking closely at the Android source for a while.

The last parameter for an ACP start isn't whether or not the other device is connected, but whether or not it is successfully streaming. The android code validates that the other side exists and that they have responded to an ACP start already. I personally don't really know what the value of this command can possibly be. The hearing devices are already supposed to be in communication with each other, and relying on the central to indicate the status of the other hearing device would be prone to race conditions.

I was wondering about this, the documentation is not very clear about whether "connected" means "streaming audio", "l2cap connection" or "connected for GATT services".

I don't exactly know the purpose of this value, but android sets it to 10

From what I understand, the connection latency is how many connection events the peripheral is allowed to skip waking up for. Only really relevant when not streaming audio.

I would defer encoding audio until the last moment before you send it. g722 is not stateless, and if you have to drop a packet, it can cause audible artifacts in the stream.

So the current architecture is that encoding doesn't start until at least one HA has sent the ACP start command and status notification has been received. I need to add reset capability on stream stop, or HA disconnection etc.

I'll also note that you haven't done anything to manage stream pacing. I've had difficulty getting the pacing right on linux because I don't have direct access to the bt stack. The android source code will preemptively drop packets if:

Apart from the pre-buffer, pacing is currently dictated by the audio coming from the USB side. USB audio uses an isochronous stream, and delivers 1ms of audio every ms. My crude measurements indicate the USB core delivers 20ms of G.722 encoded audio every 20000us +- 25us or so.

What I'm not currently doing is accounting for any sort of drift in timing between the BT and USB stacks. I'm currently using the default BTStack runloop on the Pi Pico, which is a background task using low priority interrupts. It may be better to drive the runloop myself instead. I also need to find if I can get the number of available credits in the L2CAP channel. There doesn't seem to be an API for it, but the data is probably hidden in a struct somewhere...

Again, thank you so much for your feedback.

thewierdnut commented 2 months ago

If you ever do decide to pick up a Pico W or two, feel free to test

I know nothing about this stuff... If I wanted to test this out, what would I need?

thewierdnut commented 2 months ago

USB audio uses an isochronous stream, and delivers 1ms of audio every ms

That may be sufficient. Just wait until you accumulate 160 bytes of g722 data, then deliver it.

I have the opposite problem. Pipewire delivers 170.5 samples per invocation (it alternates between 170 and 171), and I have to wait until I have an even number of samples so that I can encode them, so frames are available on a very irregular schedule. You could literally hear the frame drops phasing in and out of existence as the frame delivery times almost lined up correctly for a moment, then fell out of sync again.

shermp commented 2 months ago

I think I listed everything in the readme, but for basic testing without logs, a Pi Pico and a microusb cable is all that's required. If you want logging capability, a method for interfacing with the UART serial is also required. If you don't already have anything that can do that (like a full raspberry pi or similar SBC).

For full on development, I recommend the pre-populated header version of the Pico W (Pico WH), and the Raspberry Pi Debug Probe. The debug probe comes with all the cables for UART logging, and to connect to the SWD header which allows for non-interactive firmware/code uploading.

shermp commented 2 months ago

Note, you can also make your own debug probe using another Pico (that's essentially what the official debug probe is), but you would have to source the connectors yourself.

thewierdnut commented 2 months ago

$6? Ordered.

thewierdnut commented 2 months ago

This I think is an important project, because it could potentially support anything that supports usb audio, including older tablets, phones, or windows devices.

shermp commented 2 months ago

$6? Ordered.

I know, right? I have two, one with headers, and the other without. I can highly recommend the debug probe as well if you're like me and have zero equipment or ability for soldering.

This I think is an important project, because it could potentially support anything that supports usb audio, including older tablets, phones, or windows devices.

I'm primarily a Windows user, so that's my motivation. There's also the potential to use it at work as well. The only downside is the fact it's UAC2, so may require special drivers on older versions of Windows.

barolo commented 2 months ago

This I think is an important project, because it could potentially support anything that supports usb audio, including older tablets, phones, or windows devices.

It is important. It's basically creating an open source variant o proprietary, shitty and expensive dongles from manufacturers. Widex released one recently.

shermp commented 2 months ago

The price of Oticon streaming accessories is obscene, considering they are almost certainly using commodity hardware.

barolo commented 2 months ago

The price of Oticon streaming accessories is obscene, considering they are almost certainly using commodity hardware.

They are, and all their stuff is old. Oticon isn't even the worst, look at the Roger system from Phonak for which you have to install licenses in both your aids to even use. The only new accessories are LE A streamer from Resound and dongles from Widex.

shermp commented 2 months ago

I think I may have been a little...optimistic last evening as to where I was at.

Anyway, I've fixed pairing not working. Apparently using both cores on the Pico means you can't write to flash storage without some extra work.

I poked at trying to get the pico USB stdio working, and while I managed to get a USB serial showing in linux, nothing was printing to it :(

I think quick and dirty method of using volatile shared variables is biting me in the ass, i think it's been causing problems. I'm probably going to have to do some proper multicore sync like mutex, semaphore, FIFO or queue.

shermp commented 2 months ago

Also, I've got CI working and producing binaries.

The pico-asha.uf2 can be flashed while in BOOTSEL mode (hold the button while plugging in USB to PC, then copy the pico-asha.uf2 file to the "mass storage" folder).

The pico-asha.elf can be used with the openocd tool to flash when connected to the SWD port.

thewierdnut commented 2 months ago

It took me six months from the time I first got audio to play back on my hearing aids to the time where I got it to show up as a selectable pipewire sink without choppy audio.

Don't get frustrated. These details take a while.

shermp commented 2 months ago

If you see my git history, I started on this a long time ago, had a pretty long hiatus, then the current BlueZ activity inspired me to come back to it.

barolo commented 2 months ago

If you see my git history, I started on this a long time ago, had a pretty long hiatus, then the current BlueZ activity inspired me to come back to it.

The initial BlueZ ASHA proof of content was made more than 2 years ago, in private repo, then abandoned...

shermp commented 2 months ago

Yeah, I'll get there with this hopefully.

Let's just pretend that last weekend where I spent far too long trying to figure out ways of improving G.722 encoding performance never happened.

I was compiling in debug mode without realizing it -- oops.

shermp commented 2 months ago

Just as an update, I'm halfway through replacing the shared struct with the SDK provided queue API. I'll probably finish it tomorrow evening, if I can get it working.

shermp commented 2 months ago

Update, still trying to think of how to correctly handle streaming to two devices properly.

shermp commented 2 months ago

I pushed a branch using a queue instead of the shared ring buffer. It didn't go well. There might be some good stuff to pick out here and there.

I've made a few updates to main (which uses the shared ring buffer). So far it's my most stable effort, although with plenty of caveats.

First, you have to compile with the MinSizeRel cmake config. The normal Release seems to optimize away the volatile variable accesses (or I haven't set a variable to volatile that I should. I really need to solve this properly. I've updated the readme and CI builds.

For some reason, on start up if both HA's are available to connect, only one will start connecting, the other will disconnect. Upon reconnecting it will start streaming audio too. If the other HA is instead connected later, it starts streaming straight away.

Also, if you want to test this, be careful of volume! ASHA can get loud (at least on my aids). Around 20%-30% is more than enough for me (It's the same on my phone, so not an issue with my implementation).

shermp commented 2 months ago

On the upside, I've been streaming to both aids for at least 15 minutes now (probably longer) without either of them disconnecting.

EDIT 1: At least 30 minutes now. This is the most stable it's ever been for me. EDIT 2: Still going 50 minutes in. Gotta go to bed now (work tomorrow :( ) so can't do a longer duration test.

shermp commented 2 months ago

Final note before bed, there seems to be some slight crackling or popping on some quiet bits. Not sure if I'm hearing things or not, it;'s very subtle.

thewierdnut commented 2 months ago

If you want to arrange things into pull requests, it makes it easier to review if you want us to do that.

shermp commented 2 months ago

I'm not sure if my aids are playing very slightly out of sync or I currently have extreme stereo separation. Probably the latter, but it sounds slightly unsettling.

EDIT: Or it could sound weird because I'm not resetting the encoder when a HA connects after streaming has started...

shermp commented 2 months ago

Does anyone know if the G.722 encoder (I'm using the one from android, or one of its variants) is actually designed for 14-bit audio? I've been feeding it 16-bit, but it seems G.722 is designed for 14-bit audio.

thewierdnut commented 2 months ago

I'm not sure if my aids are playing very slightly out of sync or I currently have extreme stereo separation. Probably the latter, but it sounds slightly unsettling.

I've observed this as well. In theory the hearing aids are supposed to prevent that themselves, by communicating with each other to stay in sync, but I don't know that there is a way to make sure they are actually doing that. I think the android code makes sure that the two streams stay within 4 tokens of each other (80 ms) to try and enforce the synchronization.

Or it could sound weird because I'm not resetting the encoder when a HA connects after streaming has started...

I've observed this making popping noises in the middle of the stream, but it usually recovers pretty quickly. I don't think it can distort the audio the way you describe, but it may be worth some testing to make sure.

Does anyone know if the G.722 encoder (I'm using the one from android, or one of its variants) is actually designed for 14-bit audio?

I'm no audio encoding expert, but if you stole the g722 encoder from android, you can see two things. The first is that unless you explicitly set RUN_LIKE_REFERENCE_G722, it doesn't limit the signal to 14 bits. Presumably that means that it can get some useful information out of the extra two bits somehow.

#ifdef RUN_LIKE_REFERENCE_G722
                /* The following lines are only used to verify bit-exactness
                 * with reference implementation of G.722. Higher precision
                 * is achieved without limiting the values.
                 */
                xlow = limitValues(xlow);
                xhigh = limitValues(xhigh);
#endif

The second thing is that unless you explicitly set PACKED_OUTPUT, it ignores the bitrate you provide it, and uses the full 64kbit bandwidth. I don't know what the original version of this encoder looks like, but I think this code has been modified to get the most out of the codec.

shermp commented 2 months ago

Ok, the weird sound was definitely a sync issue. I just disabled pre-buffer (which would only happen on one side, and the weird sound goes away. My HA's actually seem pretty happy with no pre-buffer actually.

I take that back, it's definitely more unstable.

shermp commented 2 months ago

Ok, I'm going to use PR's instead of committing directly to main. First one is #2

That PR enables USB serial, so there is no longer a requirement to use UART for logging. As well as a headset, the pico also shows up as a USB serial port.

shermp commented 2 months ago

Pushed a couple of "audio-sync" branching trying some different approaches. Still struggling getting a stable in-sync stereo stream.

I may have to fork BTStack at some point to add functions to get the available credits, which might be required.