At Fri, 13 Jun 2008 19:37:48 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 13 Jun 2008, Takashi Iwai wrote:
At Fri, 13 Jun 2008 18:47:48 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 13 Jun 2008, Takashi Iwai wrote:
At Fri, 13 Jun 2008 18:11:12 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 13 Jun 2008, Takashi Iwai wrote:
What about just providing three pointers: curr_ptr, hw_ptr and appl_ptr? curr_ptr corresponds to the point being played, and hw_ptr is the point where the data was already sent to h/w, and appl_ptr is the point where the data is filled by user. The above definitions are all combinations of these pointers.
But I think that curr_ptr can be managed in drivers, thus invisible to user space (except for snd_pcm_delay() propagation).
Ditto for hw_ptr. Why is it hidden at all?
Does it improve something to show this pointer to apps? I don't see any reason to show it outside alsa-lib.
Then it'll be more clear.
Maybe for us, but not for application developers. They need to know only how much samples are available for I/O operation.
I don't agree with this fully. It really depends on the type of the application, and our analysis on this isn't enough, I'm afraid. If it were only about the available samples, this issue wouldn't be raised. But, apps want more than that, obviously. We should have more closer looking at real use cases.
If driver requires extra handling of samples, it can allocate and manage extra buffers itself. I don't see the point to have "locked" samples already processed by hardware in the main ring buffer described by appl_ptr / hw_ptr. Application can use this space for new samples.
The only advantage with your implementation might be zero-copy, but USB and PCMCIA cards have or create own buffers, so I don't think that this advantage can be used in actual drivers and I cannot even imagine hardware which work in way to use zero-copy in this situation.
Wait, wait. Please don't mix up. The above doesn't imply anything about the further implementation of usb-audio driver. What I suggested is, instead of hiding two pointers (hw_ptr and curr_ptr) and creating a complex API, simply expose them.
I don't see a reason to make current API more complex.
Because the current API is complex and hard to understand.
But this concrete part of API is quite simple, isn't?
Well, it's a question how to see the current API. I don't mind to change the semantics of the current API slightly to resolve this particular problem, i.e. make snd_pcm_avail_update() to return the hwptr while snd_pcm_delay() returns a different size containing curr_ptr. So, let it be. My suggestion is no mandatory requirement, but just answering to Lennart's proposal.
But, my suggestion should be kept in mind for designing newer APIs. Many things come from the hiding obvious things although many parts are still sticking with old Unix concept.
We have already two functions, One showing overall latency and second one how much samples can be processed by application. It's enough. We need only improve things internaly in alsa-lib <-> kernel (provide correct information for snd_pcm_delay()).
Now, regarding the usb-driver. Honestly, I don't understand what you want to do with an extra URB.
Note that we don't need to have extra URBs, just change hw_ptr handling in USB driver.
OK, then it's different from your previous explanation...
Yes, sorry for not perfect explanation. I meant this.
As now, usb-audio driver handles as curr_ptr == hw_ptr. But, in reality, curr_ptr = hw_ptr - samples_in_urbs. So, in the case of USB-audio, hw_ptr is ahead of curr_ptr. (And the granularity is samples_in_urbs).
As Lennart mentioned, in this case you can reach underrun at different position than expected (when URB cannot be filled). In my case, you'll reach underrun exactly at point when whole ring buffer is drained. So application can better estimate queueing and also it makes things more logical.
Hm, could you elaborate how to do this more exactly? That wasn't clear from your previous post at all.
Looking to USB driver, snd_period_elapsed() is called directly after URB is filled (of course when it crosses the period boundary). Also hwptr_done variable is updated in this time.
It means that the PCM midlevel code thinks that samples in URBs are played (underrun can be detected), but they are queued in URBs.
OK, my fault. It's exactly behaviour I proposed (URBs are extra buffers), but we need to take in account the right snd_pcm_delay() output. Lennart probably meant that samples are consumed too much quickly at the stream start and impossibility to detect the extra buffering mechanism with the current code.
I would propose to add an extra callback to snd_pcm_ops to determine queued samples by driver and/or in hardware and extend snd_pcm_status and snd_pcm_mmap_status structures to propagate this value to user space.
Exposing to user space can be done by snd_pcm_delay(), I think. For example, add a new field, runtime->fifo_size, which is 0 as default. snd_pcm_delay() computes the delay together with runtime->fifo_size. And, the driver changes runtime->fifo_size appropriately, even dynamically. In this way, we wouldn't need any change in kernel API but only driver internals.
Takashi