On Wed, Apr 12, 2023 at 09:23:23PM +0200, Jaroslav Kysela wrote:
On 12. 04. 23 12:33, Oswald Buddenhagen wrote:
On Tue, Apr 11, 2023 at 12:47:26PM +0200, Jaroslav Kysela wrote:
On 05. 04. 23 22:12, Oswald Buddenhagen wrote:
- else if ((snd_pcm_uframes_t) hw_avail >= runtime->boundary)
hw_avail -= runtime->boundary;If hw_avail is above runtime->boundary then the initial condition is totaly bogus. I would use snd_BUG_ON() and direct return here.
I will correct that it will make sense where hw_ptr is nearby boundary (boundary - buffer_size ... boundary - 1) and appl_ptr is cropped using boundary (0 ... buffer_size).
no, that's the case where it goes negative. because it's a subtraction, it plain cannot go over the boundary when both inputs are bounded. due to the remaining correction, it could still go "very big" in an underrun condition, as the comment in the patch says. we should discuss the implications of the latter for the snd_pcm_*_avail() functions separately (the apidoc doesn't make the contract clear at all).
But because appl_ptr can be set by application without any kernel side correction, it may be possible to check if the appl_ptr is in 0 ... boundary range before any use.
that should be rather obviously done *somewhere*, as otherwise appl_ptr can often be even slightly above 2*boundary, at which point the above correction (and many alike) wouldn't even work. but for the sake of efficiency, that should be done asap, so when it is set or the boundary changes. no?
frames = runtime->silence_threshold - noise_dist;
if ((snd_pcm_sframes_t) frames <= 0)return;The retyping does not look good here. Could we move the if before frames assignment like:
if (runtime->silence_threshold <= noise_dist) return; frames = runtime->silence_threshold - noise_dist;
dunno, i don't like it - it's more noisy and imo it loses expressiveness, as the question we're asking is "how many frames do we need to fill?".
It seems that you have answer to everything.
only to the parts that i actually reply to ...
(is the requested silence threshold fulfilled? or is the noise distance greater than the whole buffer / buffer_size?).
but why would you want to approach it that way? it's just an extra step to think through. to reinforce the point: if the compiler is any good, then your variant will be optimized into mine.
regards