[alsa-devel] [PATCH - Intervals 1/1] interval: Interpret (x x+1] correctly and return x+1
From: Timo Wischer twischer@de.adit-jv.com
Without this change an interval of (x x+1] will be interpreted as an empty interval but the right value would be x+1. This leads to a failing snd_pcm_hw_params() call which returns -EINVAL.
An example issue log is given in the following: snd_pcm_hw_params failed with err -22 (Invalid argument) ACCESS: MMAP_NONINTERLEAVED FORMAT: S16_LE SUBFORMAT: STD SAMPLE_BITS: 16 FRAME_BITS: 16 CHANNELS: 1 RATE: 16000 PERIOD_TIME: (15999 16000] PERIOD_SIZE: (255 256] PERIOD_BYTES: (510 512] PERIODS: [2 3) BUFFER_TIME: 32000 BUFFER_SIZE: 512 BUFFER_BYTES: 1024
In case of (x x+1) we have to interpret it anyway as a single value of x to compensate rounding issues. For example the period size will result in an interval of (352 353) when the period time is 16ms and the sample rate 22050 Hz (16ms * 22,05 kHz = 352,8 frames). But 352 has to be chosen to allow a buffer size of 705 (32ms * 22,05 kHz = 705,6 frames) which has to be >= 2x period size to avoid Xruns. 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 instead of a real interval further on.
This issue seems to exist since the change of 9bb985c38 ("pcm: snd_interval_refine_first/last: exclude value only if also excluded before")
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/src/pcm/interval_inline.h b/src/pcm/interval_inline.h index a68e292..5e59d72 100644 --- a/src/pcm/interval_inline.h +++ b/src/pcm/interval_inline.h @@ -51,12 +51,14 @@ INTERVAL_INLINE int snd_interval_single(const snd_interval_t *i) { assert(!snd_interval_empty(i)); return (i->min == i->max || - (i->min + 1 == i->max && i->openmax)); + (i->min + 1 == i->max && (i->openmin || i->openmax))); }
INTERVAL_INLINE int snd_interval_value(const snd_interval_t *i) { assert(snd_interval_single(i)); + if (!i->openmax) + return i->max; return i->min; }
On Thu, 18 Oct 2018 12:50:26 +0200, twischer@de.adit-jv.com wrote:
From: Timo Wischer twischer@de.adit-jv.com
Without this change an interval of (x x+1] will be interpreted as an empty interval but the right value would be x+1. This leads to a failing snd_pcm_hw_params() call which returns -EINVAL.
An example issue log is given in the following: snd_pcm_hw_params failed with err -22 (Invalid argument) ACCESS: MMAP_NONINTERLEAVED FORMAT: S16_LE SUBFORMAT: STD SAMPLE_BITS: 16 FRAME_BITS: 16 CHANNELS: 1 RATE: 16000 PERIOD_TIME: (15999 16000] PERIOD_SIZE: (255 256] PERIOD_BYTES: (510 512] PERIODS: [2 3) BUFFER_TIME: 32000 BUFFER_SIZE: 512 BUFFER_BYTES: 1024
In case of (x x+1) we have to interpret it anyway as a single value of x to compensate rounding issues. For example the period size will result in an interval of (352 353) when the period time is 16ms and the sample rate 22050 Hz (16ms * 22,05 kHz = 352,8 frames). But 352 has to be chosen to allow a buffer size of 705 (32ms * 22,05 kHz = 705,6 frames) which has to be >= 2x period size to avoid Xruns. 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 instead of a real interval further on.
This issue seems to exist since the change of 9bb985c38 ("pcm: snd_interval_refine_first/last: exclude value only if also excluded before")
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/src/pcm/interval_inline.h b/src/pcm/interval_inline.h index a68e292..5e59d72 100644 --- a/src/pcm/interval_inline.h +++ b/src/pcm/interval_inline.h @@ -51,12 +51,14 @@ INTERVAL_INLINE int snd_interval_single(const snd_interval_t *i) { assert(!snd_interval_empty(i)); return (i->min == i->max ||
(i->min + 1 == i->max && i->openmax));
(i->min + 1 == i->max && (i->openmin || i->openmax)));}
This change looks reasonable, but....
INTERVAL_INLINE int snd_interval_value(const snd_interval_t *i) { assert(snd_interval_single(i));
- if (!i->openmax)
return i->max;
This change looks risky. The snd_interval_value() might be called even if the interval isn't reduced to a single value. Rather check openmin instead.
thanks,
Takashi
participants (2)
-
Takashi Iwai -
twischer＠de.adit-jv.com