On Fri, Jun 24, 2011 at 10:52 AM, Felix Homann linuxaudio@showlabor.de wrote:
Please, make sure you have the routing "diagonal", i.e. "DIn i - Out j" is set to 0% if i !=j and 100% if i == j. I've attached a small bash script to accomplish this. After plugging the device in all playback volumes are initially at 100%. I could not notice the clicks then unless it turned the volume very very loud (I guess aplay plays back on all channels, which blurs the clicks).
Thanks, that helped. It was indeed the mixer settings that caused the distortion and hid the streaming errors. I can now hear the clicks with the current mainline driver.
Attached is a preliminary patch that implements implicit feedback mode, and it works fine for the playback channel (without any clicks now). However, there are still some corner cases that need attention, and currently, the kernel crashes when you start playback and capture streams at the same time. So please don't use it in full-duplex more for now, you have been warned.
I've investigated in many ways to implement this nicely, but it's quite tricky as it breaks the current driver layout in multiple ways, regardless which way I go.
These were the approaches:
1. Like I posted last year, I wanted to just collect the number of received frames when capturing data and modify snd_audio_next_packet_size() so that it approximates to the number of samples received in the record stream. This didn't work as easily because the streaming loop ended up queueing 0-byte packets which the hardware doesn't like at all. It will stop streaming and won't recover after the first empty packet was sent. Hence, sending data at a fix urb rate seems no way to go, at least for the FTU.
2. I tried to treat the record stream as sync endpoint and trick around with the callbacks for sync urbs, but that breaks several assumptions made in the code all over, mostly regarding the maximum packet size.
3. The most promising approach (the one implemented in the attached patch) splits the urb's size calculation from the actual ops.prepare() handler and re-uses the ops.prepare() from a new function called queue_next_out_urb(). For this, our complete_urb function must be modified to not automatically requeue the next out urb but leave it to the capture stream's retire callbacks when to fire the next out packet (and here, we will ignore 0-byte input packets and not send out 0-byte out urbs). A new counter variable (next_out_urb) is needed for that.
The problem, however, is that we now have to check when to actually start and stop the capture stream:
- We have to start it (and put it to pause mode) once the playback starts and switch over to the real callback once the user starts the capture PCM stream - We also have to start it if the user only wants to capture PCM, without playback. - We can stop it if the user was only using capture PCM or playback PCM streams. - But we cannot stop it if it still serves as feedback source for a running playback PCM.
I started to implement some logic for this, but I'm not really happy with it yet. Some reference counting would be much better than what I currently have, but I don't see a good solution yet. Maybe I've been looking at this for too long now, and someone else has an idea?
I have a split version of this patchset here locally, but I think the consolidated version is easier to test for now.
Feedback more than welcome.
Thanks, Daniel