[pulseaudio-discuss] [PATCH] alsa-util: Set ALSA report_delay flag in pa_alsa_safe_delay()

[Pierre-Louis Bossart]

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;
>