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

[Pierre-Louis Bossart]

>>> 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;

ok, I'll trust your math :-)

> 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.)

In the 3 of the calls to pcm_lib_apply_appl_ptr(), the check is done
already prior to calling that function
	if (appl_ptr >= runtime->boundary)
		appl_ptr -= runtime->boundary;
	err = pcm_lib_apply_appl_ptr(substream, appl_ptr);


it's rather unclear to me why the same check is not done for sync_ptr, e.g.

if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) {
	err = pcm_lib_apply_appl_ptr(substream,	
			     sync_ptr.c.control.appl_ptr);

if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL)) {
	err = pcm_lib_apply_appl_ptr(substream,
				scontrol.appl_ptr);

Should I add a check there, or add a check inside of
pcm_lib_apply_appl_ptr() which would be a duplicate in the majority of
cases?