Re: [PATCH v2 1/3] ALSA: pcm: introduce INFO_NO_REWINDS flag

[Takashi Iwai <tiwai@suse.de>]

On Tue, 12 Oct 2021 02:19:16 +0200,
Pierre-Louis Bossart wrote:
> 
> > For example, check snd_pcm_playback_avail() and co.  That contains a
> > couple of more condition checks and corrections due to the possible
> > boundary crossing.  (Here, runtime->boundary may differ depending on
> > 32 or 64bit context.)
> > 
> > The actual implementation of the backward move check would be slightly
> > different from those, but I hope you get my idea.
> 
> I think I do but not sure how to precisely deal with the boundary
> wrap-around.
> 
> The only suggestion I have at this point would be to compare the 'avail'
> space before and after the appl_ptr changes in pcm_lib_apply_appl_ptr().
> If the 'avail' space grows as a result of user-space changes, that
> indicates a rewind (both for capture and playback), doesn't it?

There are a few different ways, and a simple one would be to treat as
a rewind if the change isn't 0..buffer_size.  e.g.

	snd_pcm_sframes_t diff = new_ptr - old_ptr;

	if (diff >= 0) {
		if (diff > buffer_size)
			return REWIND;
	} else {
		if (boundary + diff > buffer_size)
			return REWIND;
	}
	return OK;

Or, if a rewind is defined to be -buffer_size..-1, it'd be like:

	snd_pcm_sframes_t diff = new_ptr - old_ptr;

	if (diff >= 0) {
		if (boundary - diff <= buffer_size)
			return REWIND;
	} else {
		if (-diff <= buffer_size)
			return REWIND;
	}
	return OK;

In either way, the new_ptr has to be validated beforehand that it's
within 0..boundary-1.  (old_ptr is assumed to be valid.)

And don't miss that diff is a signed value, so it must be
snd_pcm_sframes_t, not *_uframes_t.


Takashi