On 7/19/16 10:33 AM, Alan Young wrote:
On 15/07/16 21:13, Pierre-Louis Bossart wrote:
in update_delay() you apply a delta between the last timestamp and the current one and modify the runtime->delay.
Immediately after, in update_audio_tstamp() runtime->delay is used as well to compute audio_frames which in turn is used to find the audio_tstamp, on which another delta between current tstamp and last timestamp is applied.
Overall it looks like you are correcting twice for the same delay?
In update_audio_tstamp() it either usedruntime->delay, if runtime->audio_tstamp_config.report_delay is true, or applies a delta - not both.
ah yes, I did miss it in the code. maybe a comment would be nice to avoid being thrown.
I still have mixed feelings about the code, it seems to make sense for the case where the .pointer is updated at every period, but it's not using the regular BATCH definition. With the tests such as runtime->status->hw_ptr == runtime->hw_ptr_interrupt you could end-up modifying the results by a small amount for other hardware platforms depending on when the timestamp is taken (in other words scheduling latency would affect audio timestamps).
Even if this was correct, you would want to make sure the delta is positive to keep audio timestamps monotonous.
Hmm, maybe. In what circumstances could the delay ever be negative?
if your timestamps are REALTIME since they can jump backwards. The expectation is to use MONOTONOUS (or better MONOTONOUS_RAW to avoid NTP corrections), but with the ALSA API the application can choose REALTIME.