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

From: Mounesh Sutar <sutar.mounesh@gmail.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Andreas Pape <apape@de.adit-jv.com>,
	"Sutar, Mounesh" <Mounesh_Sutar@mentor.com>
Subject: Re: [PATCH 1/1] pcm:file: delegate htimestamping to slave instead of always getting real_htimestamp
Date: Tue, 28 Mar 2017 12:17:08 +0530	[thread overview]
Message-ID: <CAHRsm5MXyR6WcB-t-pcTEYjr1O_scRZ4rg2kz1k+SYHbrxp4pg@mail.gmail.com> (raw)
In-Reply-To: <s5hpoh1lxai.wl-tiwai@suse.de>

sure, Thanks.

On Tue, Mar 28, 2017 at 12:09 PM, Takashi Iwai <tiwai@suse.de> wrote:

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