Re: [PATCH - Intervals v2 1/1] interval: Interpret (x x+1] correctly and return x+1

[Takashi Iwai <tiwai@suse.de>]

On Thu, 18 Oct 2018 17:55:32 +0200,
Timo Wischer wrote:
> 
> > But what I don't understand is why
> >
> > snd_interval_value() {
> > 	if (openmin)
> > 		return min+1;
> > 	return min;
> > }
> >
> > doesn't work.  I don't mean that this is better than returning max
> > (like your v1 patch), but just want to understand the problem
> > correctly.
> In the issue case which I had I got a buffer size < 2x period size
> while I have requested 16ms period and 32ms buffer. Unfortunately this
> results in continuously under runs.
> The following audio path was used: aplay->rate->dmix->hw. aplay
> streams with 22050 Hz. Therefore we got the following intervals:
> period: 16ms * 22,05 kHz = 352,8 frames => (352 353)
> buffer: 32ms * 22,05 kHz = 705,6 frames => [705 706]

But how can it be at the first place?  (352 353) is already empty as
the frames.  The time could be kept in this representation, but the
frames must be integer.

Which order of calls did it result in so?

We know that some order of calls make the selection impossible like
the above, especially when both time and bytes/frames are mixed.

> The buffer size will not end up with an interval of (705 706) similar
> to the period size because snd_pcm_rate_hw_refine_cchange() calls
> snd_interval_floor() for the buffer size. Therefore this value will be
> interpreted as an integer interval (integer flag is set) instead of a
> real interval further on.
> 
> With your solution we would use 353 frames for period and 705 frames
> for buffer. Therefore we will get Xruns because 2*353 > 705.
> 
> >
> >
> >>> IOW, if we have a good set of unit tests to cover most of possible use
> >>> cases and this is proven to improve all these, I'll happily take it.
> >>> But unfortunately there is no good test coverage, so far.
> >> At least I have executed several tests on my end which are using
> >> different combination of ALSA plugins
> >> (plug, rate, dmix, dsnoop), sample rates, channels, period and buffer
> >> sizes on different hardware with different
> >> sound cards.
> >> I would expect that you are talking about the tests in the test
> >> directory of the alsa-lib sources. Or is there
> >> anything else? May be I could try to extend/add some tests at least to
> >> reproduce the issue of this patch.
> > I'd think of a test program linked with these functions for executing
> > only this configuration stuff without actual plugins.  Then we can
> > give a bunch of sets to evaluate the extreme cases.
> So you mean only testing the corner cases of the functions of
> src/pcm/interval_inline.h without any other alsa-lib dependencies?

Yes.


Takashi