[alsa-devel] Bug in snd_pcm_ioplug_avail_update()?

[Takashi Iwai]

On Sat, 14 Jul 2018 00:10:29 +0200,
Rob Duncan wrote:
> 
> It seems to me that there's something wrong in
> snd_pcm_ioplug_avail_update() for capture IO plugins.
> 
> Once a bunch of conditions are met it makes this call:
> 
> 	__snd_pcm_mmap_begin(pcm, &areas, &offset, &size);
> 	result = io->data->callback->transfer(io->data, areas, offset, size);
> 	if (result < 0)
> 		return result;
> 
> This transfers size frames from the IO plugin, which in general is not
> all the available data: it's limited by the amount of contiguous space
> in the mmap.
> 
> However, the function returns like this:
> 
> 	avail = snd_pcm_mmap_avail(pcm);
> 	if (avail > io->avail_max)
> 		io->avail_max = avail;
> 	return (snd_pcm_sframes_t)avail;
> 
> But this is all the available data in the IO plugin, without considering
> the contiguous space limitation. This means that there is now
> uninitialized data in the mmap, and some data still in the IO plugin
> that has yet to be transferred.
> 
> I'm running into this misbehaviour with a rate conversion plugin pulling
> data from an IO plugin.  I get chunks of old data showing up later in
> the stream, I think because the mmap buffer is not being fully written
> to.
> 
> I suspect that this avail mismatch tricks snd_pcm_rate_avail_update()
> into thinking that it can grab an integral number of periods of data
> from the mmap, which is not the case.
> 
> But I'm quite tangled up trying to follow the control flow here so I
> could certainly be mistaken.  I'd appreciate any insight into what might
> be going wrong.

Yeah, this looks like a bug.  The code there supposed to copy the
whole data that is available, while snd_pcm_mmap_begin() gives the
range only for a single copy action, so it won't fill if the region to
fill is split across the buffer boundary.

I guess we need some open-code there to achieve the whole data copy.
The avail value from snd_pcm_mmap_avail() can be moved before the
action, so that we can avoid to calculate this twice.

If you have already some fix patch, it'd be helpful.  Otherwise I'll
fix it up some time later.


thanks,

Takashi