At Tue, 28 Apr 2009 10:55:53 +0200 (CEST), Jaroslav Kysela wrote:
On Mon, 27 Apr 2009, Mark Brown wrote:
On Mon, Apr 27, 2009 at 05:09:27PM +0200, Jaroslav Kysela wrote:
Appearently, FIFO is too big. I'm working on some changes to take account fifo_size to the jiffies based check. Please, set fifo_size correctly in your driver. USB drivers with big "URB FIFOs" should be changed as well, too.
Could you expand a bit on what qualifies as "too big" here? I've not been following the thread here or the changes that caused problems.
The problem is that period_elapsed() callback checks if data are not "consumed" (playback) or "produced" (capture) too quickly using jiffies timing (with HZ/100 margin). In case of large FIFO, the data are buffered, but the real playback position is different.
Well, but the hardware accepts the data of period_size already at this point. In that sense, the driver returns the correct hwptr value, i.e. pointer callback behaves in a correct way.
The question is, if driver's pointer function should estimate more real stream position (taking in account the FIFO) in this case or if upper layers of the ALSA driver and applications should handle the situation using FIFO size value.
Right. The real problem is that hwptr doesn't represent the real playing/recording position in these hardware. But, it worked like that, thus there is no reason to break this behavior at this point. The identity of hwptr and the real position wasn't (isn't) strictly defined yet.
I would suggest this:
- implement the stream position corrections in the pointer callback - in this case applications get valid realtime timing source for A/V synchronization etc. - in case of mpc driver, another timing and interrupt source should be used to compute the pointer value not DMA engine
I don't think the pointer callback should be changed for that. We have two different pointer representations, and the pointer callback itself works as is.
- leave current fifo_size without change (there will be only small FIFO values to notify applications about small extra delays before samples reaches DAC or leaves ADC)
fifo_size field is supposed to be constant, and shouldn't be changed dynamically after open. This is a problem if the FIFO size varies depending on the parameter.
- add cache_size value to snd_pcm_hardware struct - here will be stored area of buffer which might be cached in the hardware for large FIFOs including prepared USB's URBs (the upper layer should not work with this buffer area - rewind operation etc.)
I already submitted a similar patch quite ago. Actually, it exposes the delay account in snd_pcm_status() delay calculation. The delay value can be dynamically changed.
The current test patches are found on sound unstable tree topic/pcm-delay branch. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable-2.6.git
Takashi