On 04. 02. 23 10:11, Alan Young wrote:
On 03/02/2023 18:02, Jaroslav Kysela wrote:
Thank you for your suggestion, but I think that the *whole* code for
!get_time_info in update_audio_tstamp() should be recoded. The calling of
ns_to_timespec64() is not enough to handle the boundary wraps in a decent
range (tenths years for 24x7 operation)
Yes, indeed. My ambition was unnecessarily short.
and the bellow code is dangerous for 32-bit apps / system:
if (crossed_boundary) {
snd_BUG_ON(crossed_boundary != 1);
runtime->hw_ptr_wrap += runtime->boundary;
}
I don't understand why?
>
I would probably propose to have just hw_ptr_wrap +1 counter (we can
reconstruct the frame position back by multiplication and do range check
later),
Would that really help that much? It would extend the total possible duration
but perhaps ~1523287 years(below) is sufficient.
remove snd_BUG_ON
Again, why?
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.
and improve the timespec64 calculation.
The calculation should be split to two parts (tv_sec / tv_nsec):
1) calculate seconds: (frames / rate)
2) calculate the remainder (ns): ((frames % rate) * NSEC_PER_SEC) / rate
With 64-bit integer range, we should go up to (for 384000Hz rate):
2**64 / 384000 / 3600 / 24 / 365 = ~1523287 years
Yes indeed. How about this?
static inline void snd_pcm_lib_frames_to_timespec64(u64 frames, unsigned int rate, struct timespec64 *audio_tstamp)
{
u32 remainder;
audio_tstamp->tv_sec = div_u64_rem(frames, rate, &remainder);
audio_tstamp->tv_nsec = div_u64(mul_u32_u32(remainder, NSEC_PER_SEC), rate);
Yes, this looks fine.
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
