Re: Overflow in calculating audio timestamp

[Jaroslav Kysela <perex@perex.cz>]

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@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.