Re: [PATCH] aplay: Support setting timestamp

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

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