[PATCH] aplay: Support setting timestamp

[PATCH] aplay: Support setting timestamp

[Pavel Hofman]

To allow enabling timestamp and specify its type, a new option
--tstamp-type=TYPE is added. Recognized values are none (default),
gettimeofday, monotonic, monotonic-raw.

Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
---
 aplay/aplay.1 |  4 ++++
 aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/aplay/aplay.1 b/aplay/aplay.1
index 3bba59d..d3b7dce 100644
--- a/aplay/aplay.1
+++ b/aplay/aplay.1
@@ -207,6 +207,10 @@ sampling rates, numbers of channels, period and buffer bytes/sizes/times.
 For raw device hw:X this option basically lists hardware capabilities of
 the soundcard.
 .TP
+\fI\-\-tstamp\-type=TYPE\fP
+Specifies timestamp type inside the software configuration container.
+Types are: none (default), gettimeofday, monotonic, monotonic\-raw.
+.TP
 \fI\-\-fatal\-errors\fP
 Disables recovery attempts when errors (e.g. xrun) are encountered; the
 aplay process instead aborts immediately.
diff --git a/aplay/aplay.c b/aplay/aplay.c
index 63a4e34..5d15a32 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -139,6 +139,8 @@ static int use_strftime = 0;
 volatile static int recycle_capture_file = 0;
 static long term_c_lflag = -1;
 static int dump_hw_params = 0;
+static int enable_tstamp = 0;
+static snd_pcm_tstamp_type_t tstamp_type = SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
 
 static int fd = -1;
 static off64_t pbrec_count = LLONG_MAX, fdcount;
@@ -244,6 +246,8 @@ _("Usage: %s [OPTION]... [FILE]...\n"
 "    --process-id-file   write the process ID here\n"
 "    --use-strftime      apply the strftime facility to the output file name\n"
 "    --dump-hw-params    dump hw_params of the device\n"
+"    --tstamp-type=TYPE  set timestamp (TYPE: none (default), gettimeofday,\n"
+"                        monotonic, monotonic-raw)\n"
 "    --fatal-errors      treat all errors as fatal\n"
   )
 		, command);
@@ -430,6 +434,7 @@ enum {
 	OPT_PROCESS_ID_FILE,
 	OPT_USE_STRFTIME,
 	OPT_DUMP_HWPARAMS,
+	OPT_TSTAMP_TYPE,
 	OPT_FATAL_ERRORS,
 };
 
@@ -517,6 +522,7 @@ int main(int argc, char *argv[])
 		{"use-strftime", 0, 0, OPT_USE_STRFTIME},
 		{"interactive", 0, 0, 'i'},
 		{"dump-hw-params", 0, 0, OPT_DUMP_HWPARAMS},
+		{"tstamp-type", 1, 0, OPT_TSTAMP_TYPE},
 		{"fatal-errors", 0, 0, OPT_FATAL_ERRORS},
 #ifdef CONFIG_SUPPORT_CHMAP
 		{"chmap", 1, 0, 'm'},
@@ -799,6 +805,23 @@ int main(int argc, char *argv[])
 		case OPT_DUMP_HWPARAMS:
 			dump_hw_params = 1;
 			break;
+		case OPT_TSTAMP_TYPE:
+			if (strcasecmp(optarg, "gettimeofday") == 0) {
+				enable_tstamp = 1;
+				tstamp_type = SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
+			} else if (strcasecmp(optarg, "monotonic") == 0) {
+				enable_tstamp = 1;
+				tstamp_type = SND_PCM_TSTAMP_TYPE_MONOTONIC;
+			} else if (strcasecmp(optarg, "monotonic-raw") == 0) {
+				enable_tstamp = 1;
+				tstamp_type = SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW;
+			} else if (strcasecmp(optarg, "none") == 0)
+				enable_tstamp = 0;
+			else {
+				error(_("unrecognized timestamp type %s"), optarg);
+				return 1;
+			}
+			break;
 		case OPT_FATAL_ERRORS:
 			fatal_errors = 1;
 			break;
@@ -1453,6 +1476,15 @@ static void set_params(void)
 		stop_threshold = (double) rate * stop_delay / 1000000;
 	err = snd_pcm_sw_params_set_stop_threshold(handle, swparams, stop_threshold);
 	assert(err >= 0);
+	if (enable_tstamp) {
+		err = snd_pcm_sw_params_set_tstamp_mode(handle, swparams, SND_PCM_TSTAMP_ENABLE);
+		assert(err >= 0);
+		err = snd_pcm_sw_params_set_tstamp_type(handle, swparams, tstamp_type);
+		if (err < 0) {
+			error(_("Unable to set the requested timestamp type."));
+			prg_exit(EXIT_FAILURE);
+		}
+	}
 
 	if (snd_pcm_sw_params(handle, swparams) < 0) {
 		error(_("unable to install sw params:"));
-- 
2.25.1

Re: [PATCH] aplay: Support setting timestamp

[Takashi Sakamoto]

Hi,

On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
> To allow enabling timestamp and specify its type, a new option
> --tstamp-type=TYPE is added. Recognized values are none (default),
> gettimeofday, monotonic, monotonic-raw.
> 
> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
> ---
>  aplay/aplay.1 |  4 ++++
>  aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
 
I prefer the idea to work for timestamp feature defined in ALSA PCM
interface, while I have a mixed feeling to integrate `aplay` tool, since
I have an intension to obsolete the tool with `axfer` tool with more
robust design with command argument compatibility (as much as possible).

This is not so strong request but would I ask you to work for `axfer` tool
instead of `aplay`? Then, it's preferable that the name of command
argument is decided with enough care of all of timestamp feature in ALSA
PCM interface, since we have two categories of timestamps at least; e.g.
system timestamp and audio timestamp. As long as I know, they possibly use
different clock sources, thus these two timestamps have different levels
of clock id, I think.

Of course, it's a loose accord in the community to obsolete `aplay`, and
it's easy to decide to continue aplay integration. (I'm not in leading
place of the project.) I'll be a bit happy if people take care of axfer
tool as well.


Regards

Takashi Sakamoto

> diff --git a/aplay/aplay.1 b/aplay/aplay.1
> index 3bba59d..d3b7dce 100644
> --- a/aplay/aplay.1
> +++ b/aplay/aplay.1
> @@ -207,6 +207,10 @@ sampling rates, numbers of channels, period and buffer bytes/sizes/times.
>  For raw device hw:X this option basically lists hardware capabilities of
>  the soundcard.
>  .TP
> +\fI\-\-tstamp\-type=TYPE\fP
> +Specifies timestamp type inside the software configuration container.
> +Types are: none (default), gettimeofday, monotonic, monotonic\-raw.
> +.TP
>  \fI\-\-fatal\-errors\fP
>  Disables recovery attempts when errors (e.g. xrun) are encountered; the
>  aplay process instead aborts immediately.
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index 63a4e34..5d15a32 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -139,6 +139,8 @@ static int use_strftime = 0;
>  volatile static int recycle_capture_file = 0;
>  static long term_c_lflag = -1;
>  static int dump_hw_params = 0;
> +static int enable_tstamp = 0;
> +static snd_pcm_tstamp_type_t tstamp_type = SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
>  
>  static int fd = -1;
>  static off64_t pbrec_count = LLONG_MAX, fdcount;
> @@ -244,6 +246,8 @@ _("Usage: %s [OPTION]... [FILE]...\n"
>  "    --process-id-file   write the process ID here\n"
>  "    --use-strftime      apply the strftime facility to the output file name\n"
>  "    --dump-hw-params    dump hw_params of the device\n"
> +"    --tstamp-type=TYPE  set timestamp (TYPE: none (default), gettimeofday,\n"
> +"                        monotonic, monotonic-raw)\n"
>  "    --fatal-errors      treat all errors as fatal\n"
>    )
>  		, command);
> @@ -430,6 +434,7 @@ enum {
>  	OPT_PROCESS_ID_FILE,
>  	OPT_USE_STRFTIME,
>  	OPT_DUMP_HWPARAMS,
> +	OPT_TSTAMP_TYPE,
>  	OPT_FATAL_ERRORS,
>  };
>  
> @@ -517,6 +522,7 @@ int main(int argc, char *argv[])
>  		{"use-strftime", 0, 0, OPT_USE_STRFTIME},
>  		{"interactive", 0, 0, 'i'},
>  		{"dump-hw-params", 0, 0, OPT_DUMP_HWPARAMS},
> +		{"tstamp-type", 1, 0, OPT_TSTAMP_TYPE},
>  		{"fatal-errors", 0, 0, OPT_FATAL_ERRORS},
>  #ifdef CONFIG_SUPPORT_CHMAP
>  		{"chmap", 1, 0, 'm'},
> @@ -799,6 +805,23 @@ int main(int argc, char *argv[])
>  		case OPT_DUMP_HWPARAMS:
>  			dump_hw_params = 1;
>  			break;
> +		case OPT_TSTAMP_TYPE:
> +			if (strcasecmp(optarg, "gettimeofday") == 0) {
> +				enable_tstamp = 1;
> +				tstamp_type = SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY;
> +			} else if (strcasecmp(optarg, "monotonic") == 0) {
> +				enable_tstamp = 1;
> +				tstamp_type = SND_PCM_TSTAMP_TYPE_MONOTONIC;
> +			} else if (strcasecmp(optarg, "monotonic-raw") == 0) {
> +				enable_tstamp = 1;
> +				tstamp_type = SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW;
> +			} else if (strcasecmp(optarg, "none") == 0)
> +				enable_tstamp = 0;
> +			else {
> +				error(_("unrecognized timestamp type %s"), optarg);
> +				return 1;
> +			}
> +			break;
>  		case OPT_FATAL_ERRORS:
>  			fatal_errors = 1;
>  			break;
> @@ -1453,6 +1476,15 @@ static void set_params(void)
>  		stop_threshold = (double) rate * stop_delay / 1000000;
>  	err = snd_pcm_sw_params_set_stop_threshold(handle, swparams, stop_threshold);
>  	assert(err >= 0);
> +	if (enable_tstamp) {
> +		err = snd_pcm_sw_params_set_tstamp_mode(handle, swparams, SND_PCM_TSTAMP_ENABLE);
> +		assert(err >= 0);
> +		err = snd_pcm_sw_params_set_tstamp_type(handle, swparams, tstamp_type);
> +		if (err < 0) {
> +			error(_("Unable to set the requested timestamp type."));
> +			prg_exit(EXIT_FAILURE);
> +		}
> +	}
>  
>  	if (snd_pcm_sw_params(handle, swparams) < 0) {
>  		error(_("unable to install sw params:"));
> -- 
> 2.25.1
 

Regards

Takashi Sakamoto

Re: [PATCH] aplay: Support setting timestamp

[Pavel Hofman]

Dne 16. 06. 22 v 10:13 Takashi Sakamoto napsal(a):
> 
> On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
>> To allow enabling timestamp and specify its type, a new option
>> --tstamp-type=TYPE is added. Recognized values are none (default),
>> gettimeofday, monotonic, monotonic-raw.
>>
>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
>> ---
>>   aplay/aplay.1 |  4 ++++
>>   aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>   
> I prefer the idea to work for timestamp feature defined in ALSA PCM
> interface, while I have a mixed feeling to integrate `aplay` tool, since
> I have an intension to obsolete the tool with `axfer` tool with more
> robust design with command argument compatibility (as much as possible).
> 
> This is not so strong request but would I ask you to work for `axfer` tool
> instead of `aplay`? Then, it's preferable that the name of command
> argument is decided with enough care of all of timestamp feature in ALSA
> PCM interface, since we have two categories of timestamps at least; e.g.
> system timestamp and audio timestamp. As long as I know, they possibly use
> different clock sources, thus these two timestamps have different levels
> of clock id, I think.
> 
> Of course, it's a loose accord in the community to obsolete `aplay`, and
> it's easy to decide to continue aplay integration. (I'm not in leading
> place of the project.) I'll be a bit happy if people take care of axfer
> tool as well.

Thanks for your input. I use aplay in my project and needed to have 
tstamps enabled in proc status files for my simple code which calculates 
relative samplerate between capture and playback soundcards (one of them 
having rate adjustable - audio gadget, snd-aloop) 
https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201647.html 
. The existing aplay did not have this feature, so I added it and 
submitted the patch. I did not know aplay was planned to be obsoleted, 
it seems to receive a healthy stream of patches.

As of the tstamp terminology - what command option would be more 
appropriate instead? Thanks a lot,

Pavel.

Re: [PATCH] aplay: Support setting timestamp

[Jaroslav Kysela]

On 16. 06. 22 12:00, Pavel Hofman wrote:
> Dne 16. 06. 22 v 10:13 Takashi Sakamoto napsal(a):
>>
>> On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
>>> To allow enabling timestamp and specify its type, a new option
>>> --tstamp-type=TYPE is added. Recognized values are none (default),
>>> gettimeofday, monotonic, monotonic-raw.
>>>
>>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
>>> ---
>>>    aplay/aplay.1 |  4 ++++
>>>    aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
>>>    2 files changed, 36 insertions(+)
>>    
>> I prefer the idea to work for timestamp feature defined in ALSA PCM
>> interface, while I have a mixed feeling to integrate `aplay` tool, since
>> I have an intension to obsolete the tool with `axfer` tool with more
>> robust design with command argument compatibility (as much as possible).
>>
>> This is not so strong request but would I ask you to work for `axfer` tool
>> instead of `aplay`? Then, it's preferable that the name of command
>> argument is decided with enough care of all of timestamp feature in ALSA
>> PCM interface, since we have two categories of timestamps at least; e.g.
>> system timestamp and audio timestamp. As long as I know, they possibly use
>> different clock sources, thus these two timestamps have different levels
>> of clock id, I think.
>>
>> Of course, it's a loose accord in the community to obsolete `aplay`, and
>> it's easy to decide to continue aplay integration. (I'm not in leading
>> place of the project.) I'll be a bit happy if people take care of axfer
>> tool as well.
> 
> Thanks for your input. I use aplay in my project and needed to have
> tstamps enabled in proc status files for my simple code which calculates
> relative samplerate between capture and playback soundcards (one of them
> having rate adjustable - audio gadget, snd-aloop)
> https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201647.html
> . The existing aplay did not have this feature, so I added it and
> submitted the patch. I did not know aplay was planned to be obsoleted,
> it seems to receive a healthy stream of patches.

It would be good to sync both tools (aplay/axfer) for the easy replacement. 
Could you add this code to axfer, too?

> As of the tstamp terminology - what command option would be more
> appropriate instead? Thanks a lot,

I'm fine with the proposed argument.

				Thank you,
					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH] aplay: Support setting timestamp

[Takashi Sakamoto]

Hi,

On Thu, Jun 16, 2022 at 12:00:22PM +0200, Pavel Hofman wrote:
> Dne 16. 06. 22 v 10:13 Takashi Sakamoto napsal(a):
> > 
> > On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
> > > To allow enabling timestamp and specify its type, a new option
> > > --tstamp-type=TYPE is added. Recognized values are none (default),
> > > gettimeofday, monotonic, monotonic-raw.
> > > 
> > > Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
> > > ---
> > >   aplay/aplay.1 |  4 ++++
> > >   aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
> > >   2 files changed, 36 insertions(+)
> > I prefer the idea to work for timestamp feature defined in ALSA PCM
> > interface, while I have a mixed feeling to integrate `aplay` tool, since
> > I have an intension to obsolete the tool with `axfer` tool with more
> > robust design with command argument compatibility (as much as possible).
> > 
> > This is not so strong request but would I ask you to work for `axfer` tool
> > instead of `aplay`? Then, it's preferable that the name of command
> > argument is decided with enough care of all of timestamp feature in ALSA
> > PCM interface, since we have two categories of timestamps at least; e.g.
> > system timestamp and audio timestamp. As long as I know, they possibly use
> > different clock sources, thus these two timestamps have different levels
> > of clock id, I think.
> > 
> > Of course, it's a loose accord in the community to obsolete `aplay`, and
> > it's easy to decide to continue aplay integration. (I'm not in leading
> > place of the project.) I'll be a bit happy if people take care of axfer
> > tool as well.
> 
> Thanks for your input. I use aplay in my project and needed to have tstamps
> enabled in proc status files for my simple code which calculates relative
> samplerate between capture and playback soundcards (one of them having rate
> adjustable - audio gadget, snd-aloop)
> https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201647.html

I had read your message, then replied to the patch.

I'm not at so strong objection to your patch, however if I'm allowed to
note my opinion honestly, it surely brings an issue in a point of
application design. In short, the purpose of the patch is just for the
case to retrieve the history of PCM buffer pointer position and system
time stamp via procfs node for PCM substream of aplay for analysis,
therefore it's not for aplay itself.

As you know, aplay program has no code to process time stamp except for
the case of XRUN detection. I can easily imagine the future that the new
command line option is rarely used, except at your laboratory or office.


In my opinion, the better practice for your case is to add the way to
configure parameters of PCM substream for time stamp operation; e.g. add
kobject parameter to ALSA PCM device for PCM substream (please avoid to
hack procfs node since it's ancient way in unregulated world).

Although it's probably and technically possible, it has side effect to
user processes of existent ALSA PCM applications such as PipeWire plugins
and PulseAudio modules when the applications voluntarily process time
stamp for any purpose.

> . The existing aplay did not have this feature, so I added it and submitted
> the patch. I did not know aplay was planned to be obsoleted, it seems to
> receive a healthy stream of patches.
 
> As of the tstamp terminology - what command option would be more appropriate
> instead? Thanks a lot,

It's a kind of sophisticated work, I think.

For your information, the design of several kinds of time stamp in ALSA
PCM interface (from my memo written 2020) is below:

 - system time stamp
    - Available for several purposes
        - trigger (struct snd_pcm_status.trigger_tstamp)
           - the record of time stamp at start/stop/suspend/resume/pause
             of PCM substream.
        - current (struct snd_pcm_status.tstamp)
           - the record of the latest time stamp at updating hwptr, at
             xrun and reset of PCM substream.
        - driver (struct snd_pcm_status.driver_tstamp)
           - the record of the latest time stamp when the driver operates
             PCM substream at both hardIRQ/softIRQ and process contexts
    - Multiple levels of clock_id
       - monotinic time
       - monotonic raw time
       - real time (default, gettimeofday)
    - The sampling timing at hardIRQ context is invocation of hardIRQ
      handler instead of the time of actual interrupt request, thus
      includes time jitter due to CPU-level IRQ-mask.

 - audio time stamp (struct snd_pcm_status.audio_tstamp,
                     struct snd_pcm_mmap_status.audio_tstamp)
    - timespec compensated for audio frame granularity
    - Available when audio-related hardware supports specific function
      to record time of DMA transmission or audio link.
    - Currently, implemented for Intel HDA driver in Intel CPU (Skylate
      generation or later) with Global Time Stamp (GTS) register.
    - Multiple levels
       - default
           - computed just from hwptr
       - link audio time
       - link absolute audio time
       - link estimated audio time
       - link synchronized autio time
    - For detail, please refer to
      `Documentation/sound/designs/timestamping.rst`.


Cheers

Takashi Sakamoto

Re: [PATCH] aplay: Support setting timestamp

[Pavel Hofman]

Dne 16. 06. 22 v 12:50 Jaroslav Kysela napsal(a):
> On 16. 06. 22 12:00, Pavel Hofman wrote:
>> Dne 16. 06. 22 v 10:13 Takashi Sakamoto napsal(a):
>>>
>>> On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
>>>> To allow enabling timestamp and specify its type, a new option
>>>> --tstamp-type=TYPE is added. Recognized values are none (default),
>>>> gettimeofday, monotonic, monotonic-raw.
>>>>
>>>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
>>>> ---
>>>>    aplay/aplay.1 |  4 ++++
>>>>    aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
>>>>    2 files changed, 36 insertions(+)
>>> I prefer the idea to work for timestamp feature defined in ALSA PCM
>>> interface, while I have a mixed feeling to integrate `aplay` tool, since
>>> I have an intension to obsolete the tool with `axfer` tool with more
>>> robust design with command argument compatibility (as much as possible).
>>>
>>> This is not so strong request but would I ask you to work for `axfer` 
>>> tool
>>> instead of `aplay`? Then, it's preferable that the name of command
>>> argument is decided with enough care of all of timestamp feature in ALSA
>>> PCM interface, since we have two categories of timestamps at least; e.g.
>>> system timestamp and audio timestamp. As long as I know, they 
>>> possibly use
>>> different clock sources, thus these two timestamps have different levels
>>> of clock id, I think.
>>>
>>> Of course, it's a loose accord in the community to obsolete `aplay`, and
>>> it's easy to decide to continue aplay integration. (I'm not in leading
>>> place of the project.) I'll be a bit happy if people take care of axfer
>>> tool as well.
>>
>> Thanks for your input. I use aplay in my project and needed to have
>> tstamps enabled in proc status files for my simple code which calculates
>> relative samplerate between capture and playback soundcards (one of them
>> having rate adjustable - audio gadget, snd-aloop)
>> https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201647.html 
>>
>> . The existing aplay did not have this feature, so I added it and
>> submitted the patch. I did not know aplay was planned to be obsoleted,
>> it seems to receive a healthy stream of patches.
> 
> It would be good to sync both tools (aplay/axfer) for the easy 
> replacement. Could you add this code to axfer, too?

Honestly, I do not understand the future plan for axfer vs. aplay. I 
consider aplay a diagnostics tool for alsa problems. I do not need other 
audio frameworks available when diagnosing alsa. I need the diagnostics 
tool having maximum features useful for the diagnostics, such a tool is 
not used for and aimed at general usage.

Yet I have prepared a patch for axfer which implements the very same 
feature as for aplay into its libasound backend. However, since Takashi 
objects to the implementation as is (which I do not dispute, it 
certainly is not any complex solution to the timestamping area in alsa), 
I do not intend to submit the axfer patch.

If you find the aplay patch useful, I would be happy if you accepted it. 
  If not, no problem, I will use my hacked aplay version. Since nobody 
has added such feature to aplay over a decade of its existence, my patch 
is certainly of negligible urgency. I am just a bit afraid about future 
of this simple-to-use diagnostics tool with huge user base and extensive 
online know-how in this regard.

Thanks a lot,

Pavel.

Re: [PATCH] aplay: Support setting timestamp

[Pavel Hofman]

Dne 18. 06. 22 v 10:23 Takashi Sakamoto napsal(a):
> Hi,
> 
> On Thu, Jun 16, 2022 at 12:00:22PM +0200, Pavel Hofman wrote:
>> Dne 16. 06. 22 v 10:13 Takashi Sakamoto napsal(a):
>>>
>>> On Thu, Jun 16, 2022 at 08:54:26AM +0200, Pavel Hofman wrote:
>>>> To allow enabling timestamp and specify its type, a new option
>>>> --tstamp-type=TYPE is added. Recognized values are none (default),
>>>> gettimeofday, monotonic, monotonic-raw.
>>>>
>>>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
>>>> ---
>>>>    aplay/aplay.1 |  4 ++++
>>>>    aplay/aplay.c | 32 ++++++++++++++++++++++++++++++++
>>>>    2 files changed, 36 insertions(+)
>>> I prefer the idea to work for timestamp feature defined in ALSA PCM
>>> interface, while I have a mixed feeling to integrate `aplay` tool, since
>>> I have an intension to obsolete the tool with `axfer` tool with more
>>> robust design with command argument compatibility (as much as possible).
>>>
>>> This is not so strong request but would I ask you to work for `axfer` tool
>>> instead of `aplay`? Then, it's preferable that the name of command
>>> argument is decided with enough care of all of timestamp feature in ALSA
>>> PCM interface, since we have two categories of timestamps at least; e.g.
>>> system timestamp and audio timestamp. As long as I know, they possibly use
>>> different clock sources, thus these two timestamps have different levels
>>> of clock id, I think.
>>>
>>> Of course, it's a loose accord in the community to obsolete `aplay`, and
>>> it's easy to decide to continue aplay integration. (I'm not in leading
>>> place of the project.) I'll be a bit happy if people take care of axfer
>>> tool as well.
>>
>> Thanks for your input. I use aplay in my project and needed to have tstamps
>> enabled in proc status files for my simple code which calculates relative
>> samplerate between capture and playback soundcards (one of them having rate
>> adjustable - audio gadget, snd-aloop)
>> https://mailman.alsa-project.org/pipermail/alsa-devel/2022-June/201647.html
> 
> I had read your message, then replied to the patch.
> 
> I'm not at so strong objection to your patch, however if I'm allowed to
> note my opinion honestly, it surely brings an issue in a point of
> application design. In short, the purpose of the patch is just for the
> case to retrieve the history of PCM buffer pointer position and system
> time stamp via procfs node for PCM substream of aplay for analysis,
> therefore it's not for aplay itself.
> 
> As you know, aplay program has no code to process time stamp except for
> the case of XRUN detection. I can easily imagine the future that the new
> command line option is rarely used, except at your laboratory or office.
> 
> 
> In my opinion, the better practice for your case is to add the way to
> configure parameters of PCM substream for time stamp operation; e.g. add
> kobject parameter to ALSA PCM device for PCM substream (please avoid to
> hack procfs node since it's ancient way in unregulated world).
> 
> Although it's probably and technically possible, it has side effect to
> user processes of existent ALSA PCM applications such as PipeWire plugins
> and PulseAudio modules when the applications voluntarily process time
> stamp for any purpose.
> 
>> . The existing aplay did not have this feature, so I added it and submitted
>> the patch. I did not know aplay was planned to be obsoleted, it seems to
>> receive a healthy stream of patches.
>   
>> As of the tstamp terminology - what command option would be more appropriate
>> instead? Thanks a lot,
> 
> It's a kind of sophisticated work, I think.
> 
> For your information, the design of several kinds of time stamp in ALSA
> PCM interface (from my memo written 2020) is below:
> 
>   - system time stamp
>      - Available for several purposes
>          - trigger (struct snd_pcm_status.trigger_tstamp)
>             - the record of time stamp at start/stop/suspend/resume/pause
>               of PCM substream.
>          - current (struct snd_pcm_status.tstamp)
>             - the record of the latest time stamp at updating hwptr, at
>               xrun and reset of PCM substream.
>          - driver (struct snd_pcm_status.driver_tstamp)
>             - the record of the latest time stamp when the driver operates
>               PCM substream at both hardIRQ/softIRQ and process contexts
>      - Multiple levels of clock_id
>         - monotinic time
>         - monotonic raw time
>         - real time (default, gettimeofday)
>      - The sampling timing at hardIRQ context is invocation of hardIRQ
>        handler instead of the time of actual interrupt request, thus
>        includes time jitter due to CPU-level IRQ-mask.
> 
>   - audio time stamp (struct snd_pcm_status.audio_tstamp,
>                       struct snd_pcm_mmap_status.audio_tstamp)
>      - timespec compensated for audio frame granularity
>      - Available when audio-related hardware supports specific function
>        to record time of DMA transmission or audio link.
>      - Currently, implemented for Intel HDA driver in Intel CPU (Skylate
>        generation or later) with Global Time Stamp (GTS) register.
>      - Multiple levels
>         - default
>             - computed just from hwptr
>         - link audio time
>         - link absolute audio time
>         - link estimated audio time
>         - link synchronized autio time
>      - For detail, please refer to
>        `Documentation/sound/designs/timestamping.rst`.
> 
> 

Takashi, I really do appreciate your comprehensive and very useful 
explanation. It should become part of core alsa documentation somehow, 
please.

But I cannot implement such extensive solution to either aplay or axfer, 
that's way above my skills and especially needs.

As I said in my previous message, I have axfer patch implementing the 
same feature as in my aplay patch. Should you find it usefull, I can 
submit it. You are the axfer maintainer, you call the shots :-)

Thanks a lot,

Pavel.