ALSA: dmix: pcm status incomplete

Commit Message

Dan Hordern May 6, 2015, 4:29 a.m. UTC

I require timestamp information from the soundcard and host for our
application. However, I noticed that when using "dmix:0,0" instead of
"hw:0,0", the audio timestamp and delay frames are both zero.

Looking in to the code, it seems that the 'snd_pcm_dmix_status' function
does not populate either of these. In order to expose these, I have
modified this function to get this information from the slave pcm; patch
below.

I was hoping for some feedback on this change. I am not sure if the delay
value makes sense as yet, and whether there are consequences of using the
hardware trigger timestamp etc. I am guessing the dsnoop and dshare plugins
may also have this issue, though I have not checked.

Thanks,
Dan


 }

Comments

Takashi Iwai May 20, 2015, 6:47 a.m. UTC | #1

At Wed, 6 May 2015 14:29:01 +1000,
Dan Hordern wrote:
> 
> I require timestamp information from the soundcard and host for our
> application. However, I noticed that when using "dmix:0,0" instead of
> "hw:0,0", the audio timestamp and delay frames are both zero.
> 
> Looking in to the code, it seems that the 'snd_pcm_dmix_status' function
> does not populate either of these. In order to expose these, I have
> modified this function to get this information from the slave pcm; patch
> below.
> 
> I was hoping for some feedback on this change. I am not sure if the delay
> value makes sense as yet, and whether there are consequences of using the
> hardware trigger timestamp etc. I am guessing the dsnoop and dshare plugins
> may also have this issue, though I have not checked.

The trigger_tstamp needs to be kept because you can't fetch it from
the slave PCM.  (Imagine multiple dmix streams are triggered at
different times on the same slave PCM.)

For the delay value, I think your code is OK.

And yes, please fix dsnoop and dshare similarly, too.

Could you resubmit the patch with the fixes above?


Thanks!

Takashi
> 
> Thanks,
> Dan
> 
> 
> diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
> index babde6a..a4d7fc0 100644
> --- a/src/pcm/pcm_dmix.c
> +++ b/src/pcm/pcm_dmix.c
> @@ -475,12 +475,12 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm,
> snd_pcm_status_t * status)
>                 break;
>         }
>         memset(status, 0, sizeof(*status));
> +       snd_pcm_status(dmix->spcm, status);
>         status->state = snd_pcm_dmix_state(pcm);
> -       status->trigger_tstamp = dmix->trigger_tstamp;
> -       gettimestamp(&status->tstamp, pcm->tstamp_type);
>         status->avail = snd_pcm_mmap_playback_avail(pcm);
>         status->avail_max = status->avail > dmix->avail_max ? status->avail
> : dmix->avail_max;
>         dmix->avail_max = 0;
> +       status->delay = snd_pcm_mmap_playback_delay(pcm);
>         return 0;
>  }
> _______________________________________________
> Alsa-devel mailing list
> [e-mail]
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Patch

diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index babde6a..a4d7fc0 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -475,12 +475,12 @@  static int snd_pcm_dmix_status(snd_pcm_t *pcm,
snd_pcm_status_t * status)
                break;
        }
        memset(status, 0, sizeof(*status));
+       snd_pcm_status(dmix->spcm, status);
        status->state = snd_pcm_dmix_state(pcm);
-       status->trigger_tstamp = dmix->trigger_tstamp;
-       gettimestamp(&status->tstamp, pcm->tstamp_type);
        status->avail = snd_pcm_mmap_playback_avail(pcm);
        status->avail_max = status->avail > dmix->avail_max ? status->avail
: dmix->avail_max;
        dmix->avail_max = 0;
+       status->delay = snd_pcm_mmap_playback_delay(pcm);
        return 0;