On Sat, 28 Mar 2020 23:20:21 +0100, sylvain.bertrand@gmail.com wrote:
On Sat, Mar 28, 2020 at 04:34:01PM -0500, Pierre-Louis Bossart wrote:
Using MONOTONIC_RAW is very nice on paper, until you realize you can't program a timer using the information. You can only read the timestamp and not really do much if you want to sleep/wait.
In practice, if you really really need super-precise information you'll get use rdtsc(), and apply you own formulas. And otherwise stick with MONOTONIC, it's rather unlikely you will ever notice the NTP changes. PulseAudio, CRAS and a number of Android HALs use MONOTONIC and nobody ever complained.
The pb is not about using monotonic_raw, the thing is: it is documented valid to use it which I did as expected from a naive reading of the api documentation and found those issues. I can reasonably believe it will be the case for any new alsa programmer.
For my code, in the end, I think I'll use the best "audio timestamp" I can get from the status ioctl for linear interpolation with ffmpeg timestamps.
But this is off topic here.
The topic is discussing how to fix this bug, since I had to dig a bit in alsa. It appears to me the recursive fix might be a good way, since it is done for other api functions, but I am not Jaroslav Kysela neither Takashi Iwai then far from grasping all the details of alsa.
A problem for now is that we used to allow the arbitrary parameter in sw_params because it's sw_params. Propagating an error would break this assumption, and that's the (rather only) concern when we introduce the error for an invalid tstamp type.
OTOH, although the translation of timestamp can work around this compatibility problem, it would result in an inaccurate timing that applications don't expect, either; apps set up a different tstamp type just because they want accurate timing, after all, so it'd becomes rather useless.
So, judging from the both above, I find returning an error is a better approach. Above all, it's simpler.
And for dmix, we may add a new asoundrc option to specify the tstamp type. sw_params returns an error if an incompatible tstamp type is specified. This will leave users the least possibility to use the expected tstamp type while keeping the system consistent.
thanks,
Takashi