* [PATCH] alsa-lib: Add htimestamp operation in plugin file @ 2016-11-25 11:36 sutar.mounesh 2016-11-28 19:18 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: sutar.mounesh @ 2016-11-25 11:36 UTC (permalink / raw) To: patch; +Cc: alsa-devel, Joshua Frkuska, Andreas Pape, mounesh_sutar From: Andreas Pape <apape@de.adit-jv.com> 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@de.adit-jv.com> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> --- --- 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, ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-25 11:36 [PATCH] alsa-lib: Add htimestamp operation in plugin file sutar.mounesh @ 2016-11-28 19:18 ` Takashi Iwai 2016-11-29 15:15 ` Takashi Sakamoto 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2016-11-28 19:18 UTC (permalink / raw) To: sutar.mounesh; +Cc: alsa-devel, Joshua Frkuska, Andreas Pape, mounesh_sutar On Fri, 25 Nov 2016 12:36:37 +0100, sutar.mounesh@gmail.com wrote: > > From: Andreas Pape <apape@de.adit-jv.com> > > 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@de.adit-jv.com> > Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> Applied, thanks. Takashi > --- > > --- 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, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-28 19:18 ` Takashi Iwai @ 2016-11-29 15:15 ` Takashi Sakamoto 2016-11-29 15:22 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Takashi Sakamoto @ 2016-11-29 15:15 UTC (permalink / raw) To: Takashi Iwai, sutar.mounesh Cc: alsa-devel, mounesh_sutar, Andreas Pape, Joshua Frkuska Hi, On Nov 29 2016 04:18, Takashi Iwai wrote: > On Fri, 25 Nov 2016 12:36:37 +0100, > sutar.mounesh@gmail.com wrote: >> >> From: Andreas Pape <apape@de.adit-jv.com> >> >> 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@de.adit-jv.com> >> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> > > Applied, thanks. > > > Takashi > >> --- >> >> --- 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, }; Please see below commit: pcm:file: add the missing htimestamp callback http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=68ae0c72a53704d416fe77d4c612d1f88d791e02 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. Regards Takashi Sakamoto ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-29 15:15 ` Takashi Sakamoto @ 2016-11-29 15:22 ` Takashi Iwai 2016-11-29 15:34 ` Takashi Sakamoto 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2016-11-29 15:22 UTC (permalink / raw) To: Takashi Sakamoto Cc: sutar.mounesh, alsa-devel, mounesh_sutar, Andreas Pape, Joshua Frkuska On Tue, 29 Nov 2016 16:15:57 +0100, Takashi Sakamoto wrote: > > Hi, > > On Nov 29 2016 04:18, Takashi Iwai wrote: > > On Fri, 25 Nov 2016 12:36:37 +0100, > > sutar.mounesh@gmail.com wrote: > >> > >> From: Andreas Pape <apape@de.adit-jv.com> > >> > >> 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@de.adit-jv.com> > >> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> > > > > Applied, thanks. > > > > > > Takashi > > > >> --- > >> > >> --- 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, > }; > > Please see below commit: > pcm:file: add the missing htimestamp callback > http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=68ae0c72a53704d416fe77d4c612d1f88d791e02 > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-29 15:22 ` Takashi Iwai @ 2016-11-29 15:34 ` Takashi Sakamoto 2016-11-30 7:26 ` Sutar, Mounesh 0 siblings, 1 reply; 9+ messages in thread From: Takashi Sakamoto @ 2016-11-29 15:34 UTC (permalink / raw) To: Takashi Iwai Cc: sutar.mounesh, alsa-devel, mounesh_sutar, Andreas Pape, Joshua Frkuska 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=68ae0c72a53704d416fe77d4c612d1f88d791e02 >> >> 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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-29 15:34 ` Takashi Sakamoto @ 2016-11-30 7:26 ` Sutar, Mounesh 2017-02-03 10:57 ` Sutar, Mounesh 0 siblings, 1 reply; 9+ messages in thread From: Sutar, Mounesh @ 2016-11-30 7:26 UTC (permalink / raw) To: Takashi Sakamoto, Takashi Iwai Cc: sutar.mounesh@gmail.com, alsa-devel@alsa-project.org, Andreas Pape, Frkuska, Joshua Hi, I did rebase my repo before applying this patch. It applied without any conflicts. commit 0d1d25f166a0294bf596dd2e6f55579e5e04186e Author: Andreas Pape <apape@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@de.adit-jv.com> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> M src/pcm/pcm_file.c commit 2ef7a53c31de47f5f33127a89054a661a31bd310 Author: Mengdong Lin <mengdong.lin@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@linux.intel.com> Signed-off-by: Takashi Iwai <tiwai@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@sakamocchi.jp] Sent: 29 November 2016 21:04 To: Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Frkuska, Joshua <Joshua_Frkuska@mentor.com>; Andreas Pape <apape@de.adit-jv.com>; Sutar, Mounesh <Mounesh_Sutar@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2016-11-30 7:26 ` Sutar, Mounesh @ 2017-02-03 10:57 ` Sutar, Mounesh 2017-02-27 7:54 ` Sutar, Mounesh 0 siblings, 1 reply; 9+ messages in thread From: Sutar, Mounesh @ 2017-02-03 10:57 UTC (permalink / raw) To: Takashi Sakamoto, Takashi Iwai Cc: sutar.mounesh@gmail.com, alsa-devel@alsa-project.org, Andreas Pape, Frkuska, Joshua 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@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sutar, Mounesh Sent: 30 November 2016 12:57 To: Takashi Sakamoto <o-takashi@sakamocchi.jp>; Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Andreas Pape <apape@de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska@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@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@de.adit-jv.com> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> M src/pcm/pcm_file.c commit 2ef7a53c31de47f5f33127a89054a661a31bd310 Author: Mengdong Lin <mengdong.lin@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@linux.intel.com> Signed-off-by: Takashi Iwai <tiwai@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@sakamocchi.jp] Sent: 29 November 2016 21:04 To: Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Frkuska, Joshua <Joshua_Frkuska@mentor.com>; Andreas Pape <apape@de.adit-jv.com>; Sutar, Mounesh <Mounesh_Sutar@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2017-02-03 10:57 ` Sutar, Mounesh @ 2017-02-27 7:54 ` Sutar, Mounesh 2017-02-27 8:39 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Sutar, Mounesh @ 2017-02-27 7:54 UTC (permalink / raw) To: Takashi Sakamoto, Takashi Iwai Cc: sutar.mounesh@gmail.com, alsa-devel@alsa-project.org, Andreas Pape, Frkuska, Joshua ping -----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sutar, Mounesh Sent: 03 February 2017 16:28 To: Takashi Sakamoto <o-takashi@sakamocchi.jp>; Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Andreas Pape <apape@de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska@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@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sutar, Mounesh Sent: 30 November 2016 12:57 To: Takashi Sakamoto <o-takashi@sakamocchi.jp>; Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Andreas Pape <apape@de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska@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@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@de.adit-jv.com> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> M src/pcm/pcm_file.c commit 2ef7a53c31de47f5f33127a89054a661a31bd310 Author: Mengdong Lin <mengdong.lin@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@linux.intel.com> Signed-off-by: Takashi Iwai <tiwai@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@sakamocchi.jp] Sent: 29 November 2016 21:04 To: Takashi Iwai <tiwai@suse.de> Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Frkuska, Joshua <Joshua_Frkuska@mentor.com>; Andreas Pape <apape@de.adit-jv.com>; Sutar, Mounesh <Mounesh_Sutar@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] alsa-lib: Add htimestamp operation in plugin file 2017-02-27 7:54 ` Sutar, Mounesh @ 2017-02-27 8:39 ` Takashi Iwai 0 siblings, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2017-02-27 8:39 UTC (permalink / raw) To: Sutar, Mounesh Cc: sutar.mounesh@gmail.com, alsa-devel@alsa-project.org, Frkuska, Joshua, Andreas Pape, Takashi Sakamoto 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@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sutar, Mounesh > Sent: 03 February 2017 16:28 > To: Takashi Sakamoto <o-takashi@sakamocchi.jp>; Takashi Iwai <tiwai@suse.de> > Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Andreas Pape <apape@de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska@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@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sutar, Mounesh > Sent: 30 November 2016 12:57 > To: Takashi Sakamoto <o-takashi@sakamocchi.jp>; Takashi Iwai <tiwai@suse.de> > Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Andreas Pape <apape@de.adit-jv.com>; Frkuska, Joshua <Joshua_Frkuska@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@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@de.adit-jv.com> > Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> > > M src/pcm/pcm_file.c > > commit 2ef7a53c31de47f5f33127a89054a661a31bd310 > Author: Mengdong Lin <mengdong.lin@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@linux.intel.com> > Signed-off-by: Takashi Iwai <tiwai@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@sakamocchi.jp] > Sent: 29 November 2016 21:04 > To: Takashi Iwai <tiwai@suse.de> > Cc: sutar.mounesh@gmail.com; alsa-devel@alsa-project.org; Frkuska, Joshua <Joshua_Frkuska@mentor.com>; Andreas Pape <apape@de.adit-jv.com>; Sutar, Mounesh <Mounesh_Sutar@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@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-27 8:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-25 11:36 [PATCH] alsa-lib: Add htimestamp operation in plugin file sutar.mounesh 2016-11-28 19:18 ` Takashi Iwai 2016-11-29 15:15 ` Takashi Sakamoto 2016-11-29 15:22 ` Takashi Iwai 2016-11-29 15:34 ` Takashi Sakamoto 2016-11-30 7:26 ` Sutar, Mounesh 2017-02-03 10:57 ` Sutar, Mounesh 2017-02-27 7:54 ` Sutar, Mounesh 2017-02-27 8:39 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).