Re: Overflow in calculating audio timestamp

[Alan Young <consult.awy@gmail.com>]

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?


> 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);
}

Alan.