On 06. 02. 23 9:52, Alan Young wrote:
Hi Jaroslav,
On 04/02/2023 15:40, Jaroslav Kysela wrote:
For 32-bit apps the boundary is near to UINT32_MAX (see
recalculate_boundary() function). So only one crossing point is not
enough to cover a decent time range.
There should be a better check, if the add operation crosses the U64
range for snd_BUG_ON. In my eyes, it looks safer to use counter here
and do the checks
in the function which uses this value.
I think you are misunderstanding how crossed_boundary is used. It
relates to a single call of snd_pcm_update_hw_ptr0(), which should be
called once per period, or at the very least once per buffer-size. In
its processing, it may be detected that the boundary has been crossed.
There are three separate tests that could result in this but only one
should actually happen during a single call. The snd_BUG_ON() is just to
detect (report on) a failure in that logic.
Oops, the `snd_BUG_ON(crossed_boundary != 1)` check is fine, of course. I
thought that `if (crossed_boundary)` checks for `if (runtime->hw_ptr_wrap)`
which has a different meaning. Thank you for your explanation.
None of this restricts the total number of frames that might be
processed, as a result of multiple boundary crossings.
Unfortunately, for 64-bit systems, it works only for up to 2 iterations,
because boundary is nearby LONG_MAX (see snd_pcm_hw_params()).
Changing hw_ptr_wrap to be a boundary-wrap-counter instead of its
current use as the cumulative number of frames processed at boundary
wraps would not make any useful difference.
At least, we should not have a problem even with the 64-bit boundary crossing
in the hw_ptr0 function. But given the fact, that it's impossible to reach
this limit, the fix for the timestamp calculation would be sufficient at the
moment.
And `snd_BUG_ON(runtime->hw_ptr_wrap < runtime->boundary);` check may be
useful when the hw_ptr_wrap is updated.
Thanks,
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
