Re: [PATCH] ALSA: core: Remove trigger_tstamp_latched

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 8/12/24 23:05, Zeno Endemann wrote:
> Pierre-Louis Bossart wrote on 12.08.24 19:25:
>>> * The custom timestamp there does not seem to be a meaningful
>>>    improvement over the default one; There is virtually no code in
>>>    between them, so I measured only a difference of around 300ns in a
>>>    KVM VM with ich9-intel-hda device.
>>
>> Humm, you're analyzing timestamps with a VM and a rather old device?
>> ICH9 support was added in 2014, some ten years ago. The timestamping
>> stuff is only improved with SKL+.
> 
> With more modern hardware on bare metal I would assume this difference to
> the default timestamp to be even smaller. I am not a hardware guy, so
> correct me if I'm wrong, but I would think that the unknown timing delays
> of the IO operations and internal audio hardware are orders of magnitude
> larger than even 300ns, making this improvement drown in the noise. Do you
> have some measurements of the differences with modern hardware?
> 
> Besides, the only improvement here is that the timestamp is taken slightly
> earlier, nothing fancy as far as I can tell. It seems a bit odd to me that
> the hda core is the only one that cares for this.

The timestamping is not limited to HDaudio, it just turns out that only
Intel upstreamed the solution.

> Finally, I cannot imagine what audio application needs sub-microsecond
> accuracy for the trigger timestamps. That is less than a single audio frame
> even for silly sample rates. Is this intended for some scientific use case?
> For regular audio apps I'd think most do not even care that much for the
> trigger timestamps and rather use the hw-pointer-update timestamps anyway,
> to prevent clock drifts. In my case I use only the stop trigger timestamp
> to estimate at which sample position a snd_pcm_drop happened, and don't use
> the start timestamp at all.
> 
> But these are just my possibly narrow views on this. If you really have
> valid use cases for those improved timestamps I won't insist on removing
> it.
> In fact I'd be rather interested to know if you can point me to an
> application that makes use of this.

by focusing on the trigger timestamp I think you're looking at the wrong
side of the problem. The timestamping is improved by using the same
hardware counter for the trigger AND regular timestamp during
playback/capture. If you look at a hardware counter during
playback/capture but the start position is recorded with another method,
would you agree that there's a systematic non-reproducible offset at
each run? You want the trigger and regular timestamps to be measured in
the same way to avoid measurement differences.

>>> * It creates a pitfall for hda driver writers; Calling
>>>    snd_hdac_stream_timecounter_init implicitly makes them responsible
>>>    for generating these timestamps.
>>
>> That's the point, let those drivers generate a better timestamp if they
>> can. Not sure what the problem is?
> 
> This is more of an API issue. At least to me it seems bad to sneakily
> enable
> this flag in snd_hdac_stream_timecounter_init. The documentation of it does
> not make it clear that after calling it the driver is responsible for the
> timestamps. Now I am admittedly not that deep into this code, so there may
> be a reason, but again at least to an "outsider" like me it is quite
> unclear
> why initializing the time counter also means the driver now has to
> manage the
> trigger timestamps.
> 
> If you really want this functionality to stay, maybe it would be better to
> move that out of snd_hdac_stream_timecounter_init and just make every
> driver
> that wants to manage them raise the flag explicitly themselves.

Not every hardware provides a counter than can be used for precise
timestamping, so there's no practical way to require that all solutions
use the same API. USB has e.g. no such counter, the only thing you'll
get is a 1ms counter which isn't great.

I would also refer you to the different types of timestamps that can be
taken, the wallclock counter is the simple case but there's also a
capability to get an atomic snapshop for precise measurements.

I will not disagree that most applications do not need precise
timestamping, but if you want to try to enable time-of-flight
measurements for presence or gesture detection you will need higher
sampling rates and micro-second level accuracy.