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