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
