Re: [PATCH 1/1] pcm:file: delegate htimestamping to slave instead of always getting real_htimestamp

[Takashi Iwai <tiwai@suse.de>]

On Tue, 28 Mar 2017 08:37:24 +0200,
Sutar, Mounesh wrote:
> 
> ping

No need for ping.  Please give some time for review.


Takashi

> 
> -----Original Message-----
> From: sutar.mounesh@gmail.com [mailto:sutar.mounesh@gmail.com] 
> Sent: 23 March 2017 17:11
> To: patch@alsa-project.org
> Cc: alsa-devel@alsa-project.org; Sutar, Mounesh <Mounesh_Sutar@mentor.com>; Andreas Pape <apape@de.adit-jv.com>; Mounesh Sutar <sutar.mounesh@gmail.com>
> Subject: [PATCH 1/1] pcm:file: delegate htimestamping to slave instead of always getting real_htimestamp
> 
> From: Andreas Pape <apape@de.adit-jv.com>
> 
> purpose of this fix, is to read most accurate timestamps.
> 
>  From documentation of /src/pcm/pcm.c, we can see:
>  """" \par Timestamp mode
> 
>  The timestamp mode specifies, if timestamps are activated. Currently, only #SND_PCM_TSTAMP_NONE and #SND_PCM_TSTAMP_MMAP modes are known.
>  The mmap mode means that timestamp is taken on every period time boundary. Corresponding position in the ring buffer assigned to timestamp can be obtained using #snd_pcm_htimestamp() function. """"
> 
>  As snd_pcm_generic_htimestamp() internally calls snd_pcm_htimestamp() to read time, so accurate timestamp can be read from snd_pcm_generic_htimestamp().
> 
>  Also, in case of pcm_file, if the underlying slave is hardware, then we would wish to read elapsed hardware time, as it will be the most accurate, as opposed to the elapsed wall time.
>  This will provide pcm_file with the most accurate timestamps.
> 
>  Following are the timesamps read with timestamp enabled, for with fix and without fix scenarios:
> 
>  1> With fix:
> :~#time aplay --enable-tstamp -Dhtstamp_test --period-time=5000 -v -fdat /dev/urandom Playing raw data '/dev/urandom' : Signed 16 bit Little Endian, Rate 48000 Hz, Stereo File PCM (file=/tmp/swarate_out.wav) Final file PCM (file=/tmp/swarate_out.wav) ..
> 
> Slave: Hardware PCM card 0 'imx6q-sabresd-wm8962' device 0 subdevice 0 Its setup is:
>   stream       : PLAYBACK
>   access       : RW_INTERLEAVED
>   format       : S16_LE
>   subformat    : STD
>   channels     : 2
>   rate         : 48000
>   exact rate   : 48000 (48000/1)
>   msbits       : 16
>   buffer_size  : 24000
>   period_size  : 240
>   period_time  : 5000
>   tstamp_mode  : ENABLE
>   .
>   .
> Before sleep = 142:409.807623
> After sleep = 142:409.807623
> Before sleep = 142:414.806016 (calling snd_pcm_htimestamp(handle, &avail, &tstamp_before)) sleep of 2 milisec After sleep = 142:414.806016 (calling snd_pcm_htimestamp(handle, &avail, &tstamp_after) From the above timestamps, we can see that slave has returned the same timestamps, as --period-time choosen is 5msec.
> 
> 2> Without this fix:
> The timestamps are returned with realtime value.
> 
> :~# time aplay --enable-tstamp -Dhtstamp_test --period-time=5000 -v -fdat /dev/urandom Playing raw data '/dev/urandom' : Signed 16 bit Little Endian, Rate 48000 Hz, Stereo File PCM (file=/tmp/swarate_out.wav) Final file PCM (file=/tmp/swarate_out.wav) .
> .
> Slave: Hardware PCM card 0 'imx6q-sabresd-wm8962' device 0 subdevice 0 Its setup is:
>   stream       : PLAYBACK
>   access       : RW_INTERLEAVED
>   format       : S16_LE
>   subformat    : STD
>   channels     : 2
>   rate         : 48000
>   exact rate   : 48000 (48000/1)
>   msbits       : 16
>   buffer_size  : 24000
>   period_size  : 240
>   period_time  : 5000
>   tstamp_mode  : ENABLE
> .
> .
> Before sleep = 241:136.875845 (calling snd_pcm_htimestamp(handle, &avail, &tstamp_before)) sleep of 2 milisec After sleep = 241:139.076376 (calling snd_pcm_htimestamp(handle, &avail, &tstamp_after)
> 
> We can observe here, the timestamps shows time diff of ~2ms, which is the time gap of sleep duration.
> Before sleep = 241:139.617588
> After sleep = 241:141.746845
> Before sleep = 241:142.291618
> After sleep = 241:144.406406
> Before sleep = 241:144.951421
> After sleep = 241:147.066118
> Before sleep = 241:147.623421
> After sleep = 241:149.740573
> 
> Signed-off-by: Andreas Pape <apape@de.adit-jv.com>
> Signed-off-by: Mounesh Sutar <sutar.mounesh@gmail.com>
> 
> diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 8012251..fe36a1c 100644
> --- a/src/pcm/pcm_file.c
> +++ b/src/pcm/pcm_file.c
> @@ -728,7 +728,7 @@ static const snd_pcm_fast_ops_t snd_pcm_file_fast_ops = {
>  	.poll_descriptors_count = snd_pcm_generic_poll_descriptors_count,
>  	.poll_descriptors = snd_pcm_generic_poll_descriptors,
>  	.poll_revents = snd_pcm_generic_poll_revents,
> -	.htimestamp = snd_pcm_generic_real_htimestamp,
> +	.htimestamp = snd_pcm_generic_htimestamp,
>  };
>  
>  /**
> --
> 2.7.4
>