On Wed, Nov 11, 2009 at 4:57 PM, Grant Likely grant.likely@secretlab.ca wrote:
On Wed, Nov 11, 2009 at 2:34 PM, Jon Smirl jonsmirl@gmail.com wrote:
On Wed, Nov 11, 2009 at 2:24 PM, Grant Likely grant.likely@secretlab.ca wrote:
On Wed, Nov 11, 2009 at 11:37 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Nov 11, 2009 at 11:38:06AM -0500, Jon Smirl wrote:
Providing a final valid data point to the driver would possibly even make things worse since if it were used then you'd have the equivalent race where the application has initialized some data but not yet managed to update the driver to tell it it's being handed over; if the driver
That's an under run condition.
Yes, of course - the issue is that this approach encourages them, making the system less robust if things are on the edge. The mpc5200 seems to be not just on the edge but comfortably beyond it for some reason.
I can't reproduce the issue at all as long at the dev_dbg() statement in the trigger stop path is disabled. With it enabled, I hear the problem every time. The 5200 may not be a speedy beast, but it is plenty fast enough to shut down the audio stream before stale data starts getting played out.
"fast enough" - you just said it is a race. I've been saying it is a race too.
Yes, it is a race; but not the kind that is dangerous. Audio playout is always a real-time problem; whether in the middle of a stream or at the end. If the CPU gets nailed with an unbounded latency, then there will be audible artifacts - Regardless of whether the driver knows where the end of data is or not. If it does know, then audio will stutter. If it doesn't know, then there will be repeated samples. Both are nasty to the human ear. So, making the driver do extra work to keep the extra data in sync will probably force larger minimum latencies for playout (trouble for VoIP apps) so the CPU can keep up, and won't help one iota for making audio better.
I don't think it is that much more work for ALSA to provide an accessible field indicating the end of valid data. It's already tracking appl_ptr. Appl_ptr just needs to be translated into a physical DMA buffer address and we've been making mistakes doing that translation.
The real solution is to fix the worst case latencies.
There are two options:
- Eliminate the race by developing a system to deterministically flag
the end of valid data. 2) Fudge everything around making it almost impossible to lose the race, but the race is still there.
- eliminate the unbounded latencies (fix the PSC driver and/or use a
real time kernel) 4) make sure userspace fills all the periods with silence before triggering stop. Gstreamer seems to already do this. I suspect pulseaudio does the same.
The dev_dbg() aggravates the race until it is obviously visible every time. A deterministic solution would not be impacted by the dev_dbg().
But it still wouldn't help a bit when the same latency occurs in the middle of playback.
The deterministic solution of tracking the end of valid data ensures that under run will be silent instead of playing invalid data.
g.
-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.