[alsa-devel] [PATCH] alsa-lib: Add htimestamp operation in plugin file

Takashi Iwai tiwai at suse.de
Mon Feb 27 09:39:40 CET 2017 

On Mon, 27 Feb 2017 08:54:43 +0100,
Sutar, Mounesh wrote:
> 
> ping

Better to check with the actual patch and experiment with various
configurations.  The result speaks clearer than words :)


Takashi

> 
> -----Original Message-----
> From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at alsa-project.org] On Behalf Of Sutar, Mounesh
> Sent: 03 February 2017 16:28
> To: Takashi Sakamoto <o-takashi at sakamocchi.jp>; Takashi Iwai <tiwai at suse.de>
> Cc: sutar.mounesh at gmail.com; alsa-devel at alsa-project.org; Andreas Pape <apape at de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska at mentor.com>
> Subject: Re: [alsa-devel] [PATCH] alsa-lib: Add htimestamp operation in plugin file
> 
> Hi,
> 
> We had to re-look onto this change again for possible consideration.
> 
> The existing solution htimestamp (snd_pcm_generic_real_htimestamp)  ALWAYS reads the current time, while snd_pcm_generic_htimestamp reads correct timestamp from slave including evaluation of TSTAMP_MODE.
> 
> 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. 
> 
> Let us know, your opinion on this to re-consider this change.
> 
> 
> Regards,
> Mounesh
> 
> 
> -----Original Message-----
> From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at alsa-project.org] On Behalf Of Sutar, Mounesh
> Sent: 30 November 2016 12:57
> To: Takashi Sakamoto <o-takashi at sakamocchi.jp>; Takashi Iwai <tiwai at suse.de>
> Cc: sutar.mounesh at gmail.com; alsa-devel at alsa-project.org; Andreas Pape <apape at de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska at mentor.com>
> Subject: Re: [alsa-devel] [PATCH] alsa-lib: Add htimestamp operation in plugin file
> 
> Hi,
> I did rebase my repo before applying this patch. It applied without any conflicts.
> 
> commit 0d1d25f166a0294bf596dd2e6f55579e5e04186e
> Author: Andreas Pape <apape at de.adit-jv.com>
> Date:   Wed Nov 30 12:06:14 2016 +0530
> 
>     alsa-lib: Add htimestamp operation in plugin file
>     
>     PCM operation htimestamp is not implemented in plugin file.
>     Calling snd_pcm_htimestamp() on a plugin file crashes. This scenario
>     is considered now.
>     
>     Signed-off-by: Andreas Pape <apape at de.adit-jv.com>
>     Signed-off-by: Joshua Frkuska <joshua_frkuska at mentor.com>
> 
> M       src/pcm/pcm_file.c
> 
> commit 2ef7a53c31de47f5f33127a89054a661a31bd310
> Author: Mengdong Lin <mengdong.lin at linux.intel.com>
> Date:   Fri Nov 25 13:20:02 2016 +0800
> 
>     topology: Update physical link configurations in Broadwell text conf file
>     
>     To make this conf file a better example, add configurations for the
>     physical link "Codec", same as that defined by Intel Broadwell upstream
>     machine driver.
>     
>     Signed-off-by: Mengdong Lin <mengdong.lin at linux.intel.com>
>     Signed-off-by: Takashi Iwai <tiwai at suse.de>
> 
> M       src/conf/topology/broadwell/broadwell.conf
> 
> 
> It appears as, source changes are at different locations, it didn’t report any conflicts.
> Sorry for not observing this source changes, before applying the patch and pushing it to mailing list.
> And Thanks for pointing it to included fix commit.
> 
> Regards,
> Mounesh
> 
> -----Original Message-----
> From: Takashi Sakamoto [mailto:o-takashi at sakamocchi.jp]
> Sent: 29 November 2016 21:04
> To: Takashi Iwai <tiwai at suse.de>
> Cc: sutar.mounesh at gmail.com; alsa-devel at alsa-project.org; Frkuska, Joshua <Joshua_Frkuska at mentor.com>; Andreas Pape <apape at de.adit-jv.com>; Sutar, Mounesh <Mounesh_Sutar at mentor.com>
> Subject: Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file
> 
> On 2016年11月30日 00:22, Takashi Iwai wrote:
> > On Tue, 29 Nov 2016 16:15:57 +0100,
> >>>> --- a/src/pcm/pcm_file.c	2013-07-08 14:31:36.000000000 +0200
> >>>> +++ b/src/pcm/pcm_file.c	2015-05-04 16:26:10.413615403 +0200
> >>>> @@ -698,6 +698,7 @@
> >>>>  	.readi = snd_pcm_file_readi,
> >>>>  	.readn = snd_pcm_file_readn,
> >>>>  	.avail_update = snd_pcm_generic_avail_update,
> >>>> +	.htimestamp = snd_pcm_generic_htimestamp,
> >>>>  	.mmap_commit = snd_pcm_file_mmap_commit,
> >>>>  	.poll_descriptors_count = snd_pcm_generic_poll_descriptors_count,
> >>>>  	.poll_descriptors = snd_pcm_generic_poll_descriptors,
> >>
> >> I oppose this application, because designated initialization is 
> >> already applied to the .htimestamp member.
> >>
> >> ...
> >>         .poll_descriptors = snd_pcm_generic_poll_descriptors,
> >>         .poll_revents = snd_pcm_generic_poll_revents,
> >>         .htimestamp = snd_pcm_generic_htimestamp,
> 
> Oops, 'snd_pcm_generic_real_htimestamp' is proper here...
> 
> >> };
> >>
> >> Please see below commit:
> >> pcm:file: add the missing htimestamp callback
> >> http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=68ae0c72a53704
> >> d416fe77d4c612d1f88d791e02
> >>
> >> This mistake causes below warning with -Woverride-init option.
> >> pcm_file.c:714:16: warning: initialized field overwritten [-Woverride-init]
> >>   .htimestamp = snd_pcm_generic_real_htimestamp,
> >>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> I guess mentor/ADIT developers works for former snapshot, then missed 
> >> to rebase to current mainline of alsa-lib. I suggest them to di 
> >> re-evaluation with current mainline without this patch.
> >
> > I overlooked it, too.  Now the commit got reverted.
> 
> Thanks.
> 
> 
> Takashi Sakamoto
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

More information about the Alsa-devel mailing list