search

HinTak / seeed-voicecard

This is an enhancement fork with the explicit aim of supporting current shipping Raspbian/Ubuntu kernels without requiring downgrading. Please donate at https://hintak.github.io/ if it works for you. Use vX.Y branch for kernel version vX.Y
GNU General Public License v3.0
196 stars 73 forks source link

fix channel ordering #6

Open jacopomaroli opened 1 year ago

jacopomaroli commented 1 year ago

This is an attempt to address https://github.com/respeaker/seeed-voicecard/issues/309

It's an improvement on the patch proposed here https://github.com/respeaker/seeed-voicecard/issues/309#issuecomment-989912874

As far as I understand it the idea was to change the clock in the ac108 (input) driver instead of the seeed-voicecard which combines the two.

The problem is that (as the initial change was written) it was getting rid also of the clock change for the ac101 (output) without doing it anywhere else. This lead to broken output. (at least on my respeaker 6 mic)

Also now that we're doing ac108_set_clock(0, substream, cmd, dai); in the SNDRV_PCM_TRIGGER_PAUSE_PUSH event, there's no need to call ac101_trigger(substream, cmd, dai) explicitly since this is done anyway inside the ac108_set_clock function.

I tested this code and I'm consistently getting the two loopback channels at the end of the 8 inputs.

LinLin1230 commented 1 year ago

Tested on raspberry pi 3b+ and no channel shifts bug.

Appreciate for your work.

jacopomaroli commented 1 year ago

hey @LinLin1230 , thanks! Just wanted to let you know that despite improving things a lot, it wasn't a 100% bullet proof solution. As per https://github.com/respeaker/seeed-voicecard/issues/309#issuecomment-1440018368 I ended up implementing a system to detect loopback channels by their volume when running https://github.com/voice-engine/ec .

Probably we're on the right track here but there's still some work to be done. My current speculation (I could be totally wrong) is that since all the channels are writing into the same circular buffer, there's no way to know where channel 0 start; so if you delay when you start reading, you might end up offsetting your read onto other channels. This patch makes things better by starting the clock ASAP.

But yeah, just speculations. I definitely didn't spend enough time reading and debugging the code

HinTak commented 1 year ago

Apologies, trying to change default branch to v6.1 and failed, though it might be open pulls blocking it. Turn out not be the case. Leave open until I have time to think about it.

HinTak commented 1 year ago

It is that the mobile phone version of github does not let me. Switching to desktop version went ahead.

wshanmu commented 8 months ago

Hi, I tested it with my raspberry pi 4B with 32-bit Bookwarm OS and a kernel version of 6.1. But the bug still exists.

Could you please share what OS are you using so that I can test whether the OS matters?

Thanks a lot!