On 04.04.2018 00:35, Pierre-Louis Bossart wrote:
On 4/2/18 3:14 PM, Georg Chini wrote:
On 02.04.2018 21:35, Pierre-Louis Bossart wrote:
On 04/02/2018 07:54 AM, Georg Chini wrote:
The current code does not call
snd_pcm_status_set_audio_htstamp_config()
to configure the way timestamps are updated in ALSA. This leads to
incorrect time stamps in the status object returned by
snd_pcm_status(),
so the computed latencies are wrong.
This patch uses snd_pcm_status_set_audio_htstamp_config() to set the
ALSA report_delay flag to 1 before the call to snd_pcm_status(). With
this, time stamps are updated as expected.
---
src/modules/alsa/alsa-util.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/modules/alsa/alsa-util.c
b/src/modules/alsa/alsa-util.c
index 61fb4903..b91a0e98 100644
--- a/src/modules/alsa/alsa-util.c
+++ b/src/modules/alsa/alsa-util.c
@@ -1187,6 +1187,7 @@ int pa_alsa_safe_delay(snd_pcm_t *pcm,
snd_pcm_status_t *status, snd_pcm_sframes
size_t abs_k;
int err;
snd_pcm_sframes_t avail = 0;
+ snd_pcm_audio_tstamp_config_t tstamp_config;
pa_assert(pcm);
pa_assert(delay);
@@ -1200,6 +1201,12 @@ int pa_alsa_safe_delay(snd_pcm_t *pcm,
snd_pcm_status_t *status, snd_pcm_sframes
* avail, delay and timestamp values in a single kernel call
to improve
* timer-based scheduling */
+ /* The time stamp configuration needs to be set so that the
+ * ALSA code will use the internal delay reported by the
driver */
+ tstamp_config.type_requested = 1; /* ALSA default time stamp
type */
+ tstamp_config.report_delay = 1;
+ snd_pcm_status_set_audio_htstamp_config(status, &tstamp_config);
+
are you sure it's necessary or is this possibly a misunderstanding
of what audio_tstamps are?
this command is only for the audio timestamp, and to the best of my
knowledge you are not using the results using one of the
snd_pcm_status_get_audio_htstamp_* commands
the typical usage (see alsa-lib/test/audio_time.c) is this:
snd_pcm_status_set_audio_htstamp_config(status,
audio_tstamp_config);
if ((err = snd_pcm_status(handle, status)) < 0) {
printf("Stream status error: %s\n", snd_strerror(err));
exit(0);
}
snd_pcm_status_get_trigger_htstamp(status, trigger_timestamp);
snd_pcm_status_get_htstamp(status, timestamp);
snd_pcm_status_get_audio_htstamp(status, audio_timestamp);
snd_pcm_status_get_audio_htstamp_report(status,
audio_tstamp_report);
if you are not using the _get_audio_hstamp() then the config has
essentially no effect, and the delay is available separately in the
status command as before.
if ((err = snd_pcm_status(pcm, status)) < 0)
return err;
See this bug report, why it is needed:
https://bugzilla.kernel.org/show_bug.cgi?id=199235
It finally turned out that there was not a bug but just the flag
missing.
We are using snd_pcm_status_get_htstamp() with the time smoother
to calculate sink/source latency.
Humm, that looks more like a bug in the fix (20e3f9 'ALSA: pcm: update
tstamp only in audio_tstamp changed'). I don't think we intended that
changes in the way the audio_tstamp is calculated (with or without
delay) would impact when the system timestamp is updated. I am pretty
sure we only wanted to update the timestamp when the hw_ptr changed
with this fix so as to go back to the traditional behavior before
kernel 4.1.
Can you check if just using tstamp_config.type_requested = 1; isn't
enough for PulseAudio? If not, we have two conflicting desires on when
the system timestamp should be updated.
It isn't enough. For the time stamp to be updated you need to have
report_delay set.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
