[PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

Add notion of TSTAMP_CLASS (SYSTEM/AUDIO) as per Pierre's suggestion.
Add snd_pcm_start_at()

Signed-off-by: Tim Cussins <timcussins@eml.cc>

diff --git a/include/pcm.h b/include/pcm.h
index 0655e7f..c57edca 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -323,13 +323,25 @@ typedef enum _snd_pcm_tstamp {
 	SND_PCM_TSTAMP_LAST = SND_PCM_TSTAMP_ENABLE
 } snd_pcm_tstamp_t;
 
+typedef enum _snd_pcm_tstamp_class {
+	SND_PCM_TSTAMP_CLASS_SYSTEM = 0,
+	SND_PCM_TSTAMP_CLASS_AUDIO,
+	SND_PCM_TSTAMP_CLASS_LAST = SND_PCM_TSTAMP_CLASS_AUDIO,
+} snd_pcm_tstamp_class_t;
+
 typedef enum _snd_pcm_tstamp_type {
 	SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/** gettimeofday equivalent */
-	SND_PCM_TSTAMP_TYPE_MONOTONIC,	/** posix_clock_monotonic equivalent */
+	SND_PCM_TSTAMP_TYPE_MONOTONIC,		/** posix_clock_monotonic equivalent */
 	SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/** monotonic_raw (no NTP) */
 	SND_PCM_TSTAMP_TYPE_LAST = SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
 } snd_pcm_tstamp_type_t;
 
+typedef enum _snd_pcm_audio_tstamp_type {
+	SND_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
+	SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
+	SND_PCM_AUDIO_TSTAMP_TYPE_LAST = SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
+} snd_pcm_audio_tstamp_t;
+
 /** Unsigned frames quantity */
 typedef unsigned long snd_pcm_uframes_t;
 /** Signed frames quantity */
@@ -478,6 +490,7 @@ int snd_pcm_prepare(snd_pcm_t *pcm);
 int snd_pcm_reset(snd_pcm_t *pcm);
 int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status);
 int snd_pcm_start(snd_pcm_t *pcm);
+int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t* start_time);
 int snd_pcm_drop(snd_pcm_t *pcm);
 int snd_pcm_drain(snd_pcm_t *pcm);
 int snd_pcm_pause(snd_pcm_t *pcm, int enable);
diff --git a/include/sound/asound.h b/include/sound/asound.h
index 1f23cd6..1592160 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -466,12 +466,30 @@ struct snd_xfern {
 };
 
 enum {
+	SNDRV_PCM_TSTAMP_CLASS_SYSTEM = 0,
+	SNDRV_PCM_TSTAMP_CLASS_AUDIO,
+	SNDRV_PCM_TSTAMP_CLASS_LAST = SNDRV_PCM_TSTAMP_CLASS_AUDIO,
+};
+
+enum {
 	SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/* gettimeofday equivalent */
 	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC,	/* posix_clock_monotonic equivalent */
-	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,    /* monotonic_raw (no NTP) */
+	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/* monotonic_raw (no NTP) */
 	SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
 };
 
+enum {
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LAST = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
+};
+
+struct snd_start_at {
+	int tstamp_class;
+	int tstamp_type;
+	struct timespec start_time;
+};
+
 /* channel positions */
 enum {
 	SNDRV_CHMAP_UNKNOWN = 0,
@@ -550,6 +568,8 @@ enum {
 #define SNDRV_PCM_IOCTL_READN_FRAMES	_IOR('A', 0x53, struct snd_xfern)
 #define SNDRV_PCM_IOCTL_LINK		_IOW('A', 0x60, int)
 #define SNDRV_PCM_IOCTL_UNLINK		_IO('A', 0x61)
+#define SNDRV_PCM_IOCTL_START_AT	_IOW('A', 0x62, struct snd_start_at)
+
 
 /*****************************************************************************
  *                                                                           *
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index baa47c7..40a4689 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -1085,6 +1085,37 @@ int snd_pcm_start(snd_pcm_t *pcm)
 }
 
 /**
+ * \brief Start a PCM at a specified point in the future
+ * \param pcm PCM handle
+ * \param tstamp_class specifies the class of tstamp_type
+ * \param tstamp_type specifies the clock with which to interpret \p start_time
+ * \param start_time Absolute time at which to start the stream
+ * \return 0 on success otherwise a negative error code
+ * \retval -ENOSYS operation not supported for the current timestamp type
+ * \retval -EINVAL timespec, tstamp_class or tstamp_type is invalid
+ * \retval -ETIME requested start_time cannot be satisfied
+ *
+ * This method is non-blocking: It establishes an appropriate timer in the kernel
+ * that will start the stream on expiry.
+ *
+ * The timer is unconditionally cancelled upon any \a attempt to change the stream
+ * state e.g. drop, drain, start, start_at.
+ */
+int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
+{
+	assert(pcm);
+	assert(start_time);
+	if (CHECK_SANITY(! pcm->setup)) {
+		SNDMSG("PCM not set up");
+		return -EIO;
+	}
+	if (pcm->fast_ops->start_at) {
+		return pcm->fast_ops->start_at(pcm->fast_op_arg, tstamp_class, tstamp_type, start_time);
+	}
+	return -EINVAL;
+}
+
+/**
  * \brief Stop a PCM dropping pending frames
  * \param pcm PCM handle
  * \return 0 on success otherwise a negative error code
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index c34b766..0190787 100644
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -620,6 +620,26 @@ static int snd_pcm_hw_start(snd_pcm_t *pcm)
 	return 0;
 }
 
+static int snd_pcm_hw_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
+{
+	snd_pcm_hw_t *hw = pcm->private_data;
+	int err;
+
+	struct snd_start_at start_at = {
+		.tstamp_class = tstamp_class,
+		.tstamp_type = tstamp_type,
+		.start_time = *start_time
+	};
+
+	sync_ptr(hw, 0);
+	if (ioctl(hw->fd, SNDRV_PCM_IOCTL_START_AT, &start_at) < 0) {
+		err = -errno;
+		SYSMSG("SNDRV_PCM_IOCTL_START_AT failed (%i)", err);
+		return err;
+	}
+	return 0;
+}
+
 static int snd_pcm_hw_drop(snd_pcm_t *pcm)
 {
 	snd_pcm_hw_t *hw = pcm->private_data;
@@ -1336,6 +1356,7 @@ static const snd_pcm_fast_ops_t snd_pcm_hw_fast_ops = {
 	.prepare = snd_pcm_hw_prepare,
 	.reset = snd_pcm_hw_reset,
 	.start = snd_pcm_hw_start,
+	.start_at = snd_pcm_hw_start_at,
 	.drop = snd_pcm_hw_drop,
 	.drain = snd_pcm_hw_drain,
 	.pause = snd_pcm_hw_pause,
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 394505f..5cdfd3a 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -154,6 +154,7 @@ typedef struct {
 	int (*prepare)(snd_pcm_t *pcm);
 	int (*reset)(snd_pcm_t *pcm);
 	int (*start)(snd_pcm_t *pcm);
+	int (*start_at)(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time);
 	int (*drop)(snd_pcm_t *pcm);
 	int (*drain)(snd_pcm_t *pcm);
 	int (*pause)(snd_pcm_t *pcm, int enable);
-- 
1.7.10.4

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Raymond Yau]

>
> Add notion of TSTAMP_CLASS (SYSTEM/AUDIO) as per Pierre's suggestion.
> Add snd_pcm_start_at()
>

Do your implementation need to set specific start threshold to prevent the
driver automatically start when you fill the buffer ?

Do the driver allow to start when there is no data ?

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

Hi Raymond,

Thanks for the feedback :)

On 18/12/14 01:05, Raymond Yau wrote:
>  >
>  > Add notion of TSTAMP_CLASS (SYSTEM/AUDIO) as per Pierre's suggestion.
>  > Add snd_pcm_start_at()
>  >
>
> Do your implementation need to set specific start threshold to prevent
> the driver automatically start when you fill the buffer ?


> Do the driver allow to start when there is no data ?
>

It's the responsibility of the client to set the start threshold to a 
safe and responsible value.

It might suit some applications to allow both threshold start _and_ 
start_at: My implementation doesn't preclude this.

Cheers,
Tim

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

Hi all,

I posted this V2 patch a few weeks ago, it provoked a little chat but 
not much else :)

It probably looked like dumped it on the list just before the holiday - 
that's what happened, and it was a pretty crap move. There was some 
pressure at this end, and it was all a bit awkward ;) Anyways, I know I 
didn't handle that the right way.

So, in the spirit of a brighter 2015, I'm back to get start_at 
functionality merged! For fun and profit, etc.

The second patch was much better than the first (naturally :P), but 
didn't feature too much in the way of documentation and explanation, 
which was a big misstep.

If no-one has any major objections, I'll amend the V2 patch to include a 
clearer explanation of the implementation, how to use start_at, and why 
it should be merged with open arms! ;)

If you have any further suggestions about how to get code like this 
merged, I'm all ears.

Thanks,
Tim


On 17/12/14 17:27, Tim Cussins wrote:
> Add notion of TSTAMP_CLASS (SYSTEM/AUDIO) as per Pierre's suggestion.
> Add snd_pcm_start_at()
>
> Signed-off-by: Tim Cussins <timcussins@eml.cc>
>
> diff --git a/include/pcm.h b/include/pcm.h
> index 0655e7f..c57edca 100644
> --- a/include/pcm.h
> +++ b/include/pcm.h
> @@ -323,13 +323,25 @@ typedef enum _snd_pcm_tstamp {
>   	SND_PCM_TSTAMP_LAST = SND_PCM_TSTAMP_ENABLE
>   } snd_pcm_tstamp_t;
>
> +typedef enum _snd_pcm_tstamp_class {
> +	SND_PCM_TSTAMP_CLASS_SYSTEM = 0,
> +	SND_PCM_TSTAMP_CLASS_AUDIO,
> +	SND_PCM_TSTAMP_CLASS_LAST = SND_PCM_TSTAMP_CLASS_AUDIO,
> +} snd_pcm_tstamp_class_t;
> +
>   typedef enum _snd_pcm_tstamp_type {
>   	SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/** gettimeofday equivalent */
> -	SND_PCM_TSTAMP_TYPE_MONOTONIC,	/** posix_clock_monotonic equivalent */
> +	SND_PCM_TSTAMP_TYPE_MONOTONIC,		/** posix_clock_monotonic equivalent */
>   	SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/** monotonic_raw (no NTP) */
>   	SND_PCM_TSTAMP_TYPE_LAST = SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
>   } snd_pcm_tstamp_type_t;
>
> +typedef enum _snd_pcm_audio_tstamp_type {
> +	SND_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
> +	SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +	SND_PCM_AUDIO_TSTAMP_TYPE_LAST = SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +} snd_pcm_audio_tstamp_t;
> +
>   /** Unsigned frames quantity */
>   typedef unsigned long snd_pcm_uframes_t;
>   /** Signed frames quantity */
> @@ -478,6 +490,7 @@ int snd_pcm_prepare(snd_pcm_t *pcm);
>   int snd_pcm_reset(snd_pcm_t *pcm);
>   int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status);
>   int snd_pcm_start(snd_pcm_t *pcm);
> +int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t* start_time);
>   int snd_pcm_drop(snd_pcm_t *pcm);
>   int snd_pcm_drain(snd_pcm_t *pcm);
>   int snd_pcm_pause(snd_pcm_t *pcm, int enable);
> diff --git a/include/sound/asound.h b/include/sound/asound.h
> index 1f23cd6..1592160 100644
> --- a/include/sound/asound.h
> +++ b/include/sound/asound.h
> @@ -466,12 +466,30 @@ struct snd_xfern {
>   };
>
>   enum {
> +	SNDRV_PCM_TSTAMP_CLASS_SYSTEM = 0,
> +	SNDRV_PCM_TSTAMP_CLASS_AUDIO,
> +	SNDRV_PCM_TSTAMP_CLASS_LAST = SNDRV_PCM_TSTAMP_CLASS_AUDIO,
> +};
> +
> +enum {
>   	SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/* gettimeofday equivalent */
>   	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC,	/* posix_clock_monotonic equivalent */
> -	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,    /* monotonic_raw (no NTP) */
> +	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/* monotonic_raw (no NTP) */
>   	SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
>   };
>
> +enum {
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LAST = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +};
> +
> +struct snd_start_at {
> +	int tstamp_class;
> +	int tstamp_type;
> +	struct timespec start_time;
> +};
> +
>   /* channel positions */
>   enum {
>   	SNDRV_CHMAP_UNKNOWN = 0,
> @@ -550,6 +568,8 @@ enum {
>   #define SNDRV_PCM_IOCTL_READN_FRAMES	_IOR('A', 0x53, struct snd_xfern)
>   #define SNDRV_PCM_IOCTL_LINK		_IOW('A', 0x60, int)
>   #define SNDRV_PCM_IOCTL_UNLINK		_IO('A', 0x61)
> +#define SNDRV_PCM_IOCTL_START_AT	_IOW('A', 0x62, struct snd_start_at)
> +
>
>   /*****************************************************************************
>    *                                                                           *
> diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
> index baa47c7..40a4689 100644
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -1085,6 +1085,37 @@ int snd_pcm_start(snd_pcm_t *pcm)
>   }
>
>   /**
> + * \brief Start a PCM at a specified point in the future
> + * \param pcm PCM handle
> + * \param tstamp_class specifies the class of tstamp_type
> + * \param tstamp_type specifies the clock with which to interpret \p start_time
> + * \param start_time Absolute time at which to start the stream
> + * \return 0 on success otherwise a negative error code
> + * \retval -ENOSYS operation not supported for the current timestamp type
> + * \retval -EINVAL timespec, tstamp_class or tstamp_type is invalid
> + * \retval -ETIME requested start_time cannot be satisfied
> + *
> + * This method is non-blocking: It establishes an appropriate timer in the kernel
> + * that will start the stream on expiry.
> + *
> + * The timer is unconditionally cancelled upon any \a attempt to change the stream
> + * state e.g. drop, drain, start, start_at.
> + */
> +int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
> +{
> +	assert(pcm);
> +	assert(start_time);
> +	if (CHECK_SANITY(! pcm->setup)) {
> +		SNDMSG("PCM not set up");
> +		return -EIO;
> +	}
> +	if (pcm->fast_ops->start_at) {
> +		return pcm->fast_ops->start_at(pcm->fast_op_arg, tstamp_class, tstamp_type, start_time);
> +	}
> +	return -EINVAL;
> +}
> +
> +/**
>    * \brief Stop a PCM dropping pending frames
>    * \param pcm PCM handle
>    * \return 0 on success otherwise a negative error code
> diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
> index c34b766..0190787 100644
> --- a/src/pcm/pcm_hw.c
> +++ b/src/pcm/pcm_hw.c
> @@ -620,6 +620,26 @@ static int snd_pcm_hw_start(snd_pcm_t *pcm)
>   	return 0;
>   }
>
> +static int snd_pcm_hw_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
> +{
> +	snd_pcm_hw_t *hw = pcm->private_data;
> +	int err;
> +
> +	struct snd_start_at start_at = {
> +		.tstamp_class = tstamp_class,
> +		.tstamp_type = tstamp_type,
> +		.start_time = *start_time
> +	};
> +
> +	sync_ptr(hw, 0);
> +	if (ioctl(hw->fd, SNDRV_PCM_IOCTL_START_AT, &start_at) < 0) {
> +		err = -errno;
> +		SYSMSG("SNDRV_PCM_IOCTL_START_AT failed (%i)", err);
> +		return err;
> +	}
> +	return 0;
> +}
> +
>   static int snd_pcm_hw_drop(snd_pcm_t *pcm)
>   {
>   	snd_pcm_hw_t *hw = pcm->private_data;
> @@ -1336,6 +1356,7 @@ static const snd_pcm_fast_ops_t snd_pcm_hw_fast_ops = {
>   	.prepare = snd_pcm_hw_prepare,
>   	.reset = snd_pcm_hw_reset,
>   	.start = snd_pcm_hw_start,
> +	.start_at = snd_pcm_hw_start_at,
>   	.drop = snd_pcm_hw_drop,
>   	.drain = snd_pcm_hw_drain,
>   	.pause = snd_pcm_hw_pause,
> diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
> index 394505f..5cdfd3a 100644
> --- a/src/pcm/pcm_local.h
> +++ b/src/pcm/pcm_local.h
> @@ -154,6 +154,7 @@ typedef struct {
>   	int (*prepare)(snd_pcm_t *pcm);
>   	int (*reset)(snd_pcm_t *pcm);
>   	int (*start)(snd_pcm_t *pcm);
> +	int (*start_at)(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time);
>   	int (*drop)(snd_pcm_t *pcm);
>   	int (*drain)(snd_pcm_t *pcm);
>   	int (*pause)(snd_pcm_t *pcm, int enable);
>

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

Takashi, Pierre,

I didn't make it clear that I reworked the patch so that it was 
compatible with, but not dependent on, Pierre's work (viz: Timestamping 
Evolutions). In particular you'll see that I lifted 
snd_pcm_audio_tstamp_t from his patch: This means that our patches can 
be discussed/merged independently. Was this a reasonable approach?

Thanks,
Tim

On 17/12/14 17:27, Tim Cussins wrote:
> Add notion of TSTAMP_CLASS (SYSTEM/AUDIO) as per Pierre's suggestion.
> Add snd_pcm_start_at()
>
> Signed-off-by: Tim Cussins <timcussins@eml.cc>
>
> diff --git a/include/pcm.h b/include/pcm.h
> index 0655e7f..c57edca 100644
> --- a/include/pcm.h
> +++ b/include/pcm.h
> @@ -323,13 +323,25 @@ typedef enum _snd_pcm_tstamp {
>   	SND_PCM_TSTAMP_LAST = SND_PCM_TSTAMP_ENABLE
>   } snd_pcm_tstamp_t;
>
> +typedef enum _snd_pcm_tstamp_class {
> +	SND_PCM_TSTAMP_CLASS_SYSTEM = 0,
> +	SND_PCM_TSTAMP_CLASS_AUDIO,
> +	SND_PCM_TSTAMP_CLASS_LAST = SND_PCM_TSTAMP_CLASS_AUDIO,
> +} snd_pcm_tstamp_class_t;
> +
>   typedef enum _snd_pcm_tstamp_type {
>   	SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/** gettimeofday equivalent */
> -	SND_PCM_TSTAMP_TYPE_MONOTONIC,	/** posix_clock_monotonic equivalent */
> +	SND_PCM_TSTAMP_TYPE_MONOTONIC,		/** posix_clock_monotonic equivalent */
>   	SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/** monotonic_raw (no NTP) */
>   	SND_PCM_TSTAMP_TYPE_LAST = SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
>   } snd_pcm_tstamp_type_t;
>
> +typedef enum _snd_pcm_audio_tstamp_type {
> +	SND_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
> +	SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +	SND_PCM_AUDIO_TSTAMP_TYPE_LAST = SND_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +} snd_pcm_audio_tstamp_t;
> +
>   /** Unsigned frames quantity */
>   typedef unsigned long snd_pcm_uframes_t;
>   /** Signed frames quantity */
> @@ -478,6 +490,7 @@ int snd_pcm_prepare(snd_pcm_t *pcm);
>   int snd_pcm_reset(snd_pcm_t *pcm);
>   int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status);
>   int snd_pcm_start(snd_pcm_t *pcm);
> +int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t* start_time);
>   int snd_pcm_drop(snd_pcm_t *pcm);
>   int snd_pcm_drain(snd_pcm_t *pcm);
>   int snd_pcm_pause(snd_pcm_t *pcm, int enable);
> diff --git a/include/sound/asound.h b/include/sound/asound.h
> index 1f23cd6..1592160 100644
> --- a/include/sound/asound.h
> +++ b/include/sound/asound.h
> @@ -466,12 +466,30 @@ struct snd_xfern {
>   };
>
>   enum {
> +	SNDRV_PCM_TSTAMP_CLASS_SYSTEM = 0,
> +	SNDRV_PCM_TSTAMP_CLASS_AUDIO,
> +	SNDRV_PCM_TSTAMP_CLASS_LAST = SNDRV_PCM_TSTAMP_CLASS_AUDIO,
> +};
> +
> +enum {
>   	SNDRV_PCM_TSTAMP_TYPE_GETTIMEOFDAY = 0,	/* gettimeofday equivalent */
>   	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC,	/* posix_clock_monotonic equivalent */
> -	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,    /* monotonic_raw (no NTP) */
> +	SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,	/* monotonic_raw (no NTP) */
>   	SNDRV_PCM_TSTAMP_TYPE_LAST = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW,
>   };
>
> +enum {
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 0,
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LAST = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK,
> +};
> +
> +struct snd_start_at {
> +	int tstamp_class;
> +	int tstamp_type;
> +	struct timespec start_time;
> +};
> +
>   /* channel positions */
>   enum {
>   	SNDRV_CHMAP_UNKNOWN = 0,
> @@ -550,6 +568,8 @@ enum {
>   #define SNDRV_PCM_IOCTL_READN_FRAMES	_IOR('A', 0x53, struct snd_xfern)
>   #define SNDRV_PCM_IOCTL_LINK		_IOW('A', 0x60, int)
>   #define SNDRV_PCM_IOCTL_UNLINK		_IO('A', 0x61)
> +#define SNDRV_PCM_IOCTL_START_AT	_IOW('A', 0x62, struct snd_start_at)
> +
>
>   /*****************************************************************************
>    *                                                                           *
> diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
> index baa47c7..40a4689 100644
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -1085,6 +1085,37 @@ int snd_pcm_start(snd_pcm_t *pcm)
>   }
>
>   /**
> + * \brief Start a PCM at a specified point in the future
> + * \param pcm PCM handle
> + * \param tstamp_class specifies the class of tstamp_type
> + * \param tstamp_type specifies the clock with which to interpret \p start_time
> + * \param start_time Absolute time at which to start the stream
> + * \return 0 on success otherwise a negative error code
> + * \retval -ENOSYS operation not supported for the current timestamp type
> + * \retval -EINVAL timespec, tstamp_class or tstamp_type is invalid
> + * \retval -ETIME requested start_time cannot be satisfied
> + *
> + * This method is non-blocking: It establishes an appropriate timer in the kernel
> + * that will start the stream on expiry.
> + *
> + * The timer is unconditionally cancelled upon any \a attempt to change the stream
> + * state e.g. drop, drain, start, start_at.
> + */
> +int snd_pcm_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
> +{
> +	assert(pcm);
> +	assert(start_time);
> +	if (CHECK_SANITY(! pcm->setup)) {
> +		SNDMSG("PCM not set up");
> +		return -EIO;
> +	}
> +	if (pcm->fast_ops->start_at) {
> +		return pcm->fast_ops->start_at(pcm->fast_op_arg, tstamp_class, tstamp_type, start_time);
> +	}
> +	return -EINVAL;
> +}
> +
> +/**
>    * \brief Stop a PCM dropping pending frames
>    * \param pcm PCM handle
>    * \return 0 on success otherwise a negative error code
> diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
> index c34b766..0190787 100644
> --- a/src/pcm/pcm_hw.c
> +++ b/src/pcm/pcm_hw.c
> @@ -620,6 +620,26 @@ static int snd_pcm_hw_start(snd_pcm_t *pcm)
>   	return 0;
>   }
>
> +static int snd_pcm_hw_start_at(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time)
> +{
> +	snd_pcm_hw_t *hw = pcm->private_data;
> +	int err;
> +
> +	struct snd_start_at start_at = {
> +		.tstamp_class = tstamp_class,
> +		.tstamp_type = tstamp_type,
> +		.start_time = *start_time
> +	};
> +
> +	sync_ptr(hw, 0);
> +	if (ioctl(hw->fd, SNDRV_PCM_IOCTL_START_AT, &start_at) < 0) {
> +		err = -errno;
> +		SYSMSG("SNDRV_PCM_IOCTL_START_AT failed (%i)", err);
> +		return err;
> +	}
> +	return 0;
> +}
> +
>   static int snd_pcm_hw_drop(snd_pcm_t *pcm)
>   {
>   	snd_pcm_hw_t *hw = pcm->private_data;
> @@ -1336,6 +1356,7 @@ static const snd_pcm_fast_ops_t snd_pcm_hw_fast_ops = {
>   	.prepare = snd_pcm_hw_prepare,
>   	.reset = snd_pcm_hw_reset,
>   	.start = snd_pcm_hw_start,
> +	.start_at = snd_pcm_hw_start_at,
>   	.drop = snd_pcm_hw_drop,
>   	.drain = snd_pcm_hw_drain,
>   	.pause = snd_pcm_hw_pause,
> diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
> index 394505f..5cdfd3a 100644
> --- a/src/pcm/pcm_local.h
> +++ b/src/pcm/pcm_local.h
> @@ -154,6 +154,7 @@ typedef struct {
>   	int (*prepare)(snd_pcm_t *pcm);
>   	int (*reset)(snd_pcm_t *pcm);
>   	int (*start)(snd_pcm_t *pcm);
> +	int (*start_at)(snd_pcm_t *pcm, snd_pcm_tstamp_class_t tstamp_class, int tstamp_type, const snd_htimestamp_t *start_time);
>   	int (*drop)(snd_pcm_t *pcm);
>   	int (*drain)(snd_pcm_t *pcm);
>   	int (*pause)(snd_pcm_t *pcm, int enable);
>

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Pierre-Louis Bossart]

>> Do your implementation need to set specific start threshold to prevent
>> the driver automatically start when you fill the buffer ?
>
>
>> Do the driver allow to start when there is no data ?
>>
>
> It's the responsibility of the client to set the start threshold to a
> safe and responsible value.
>
> It might suit some applications to allow both threshold start _and_
> start_at: My implementation doesn't preclude this.

Now I am confused... My understanding was that this feature is similar 
to SSYNC in HDAudio, where everything is ready, buffers filled, DMAs 
armed, FIFOs filled and the start condition only opens the last gate at 
a specific time - possibly with multiple streams starting at the same 
time. If you add a condition on the start_threshold you really don't 
need any hardware-driven start, do you?
Thanks,
-Pierre

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Pierre-Louis Bossart]

> I didn't make it clear that I reworked the patch so that it was
> compatible with, but not dependent on, Pierre's work (viz: Timestamping
> Evolutions). In particular you'll see that I lifted
> snd_pcm_audio_tstamp_t from his patch: This means that our patches can
> be discussed/merged independently. Was this a reasonable approach?

Fine with me. There may be another dependency though, you probably need 
to provide INFO fields and the ability to query whether the hardware 
supports this functionality (which brings us back to the discussion on 
where to store those fields..)

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

On 06/01/15 14:42, Pierre-Louis Bossart wrote:
>
>>> Do your implementation need to set specific start threshold to prevent
>>> the driver automatically start when you fill the buffer ?
>>
>>
>>> Do the driver allow to start when there is no data ?
>>>
>>
>> It's the responsibility of the client to set the start threshold to a
>> safe and responsible value.
>>
>> It might suit some applications to allow both threshold start _and_
>> start_at: My implementation doesn't preclude this.
>
> Now I am confused... My understanding was that this feature is similar
> to SSYNC in HDAudio, where everything is ready, buffers filled, DMAs
> armed, FIFOs filled and the start condition only opens the last gate at
> a specific time - possibly with multiple streams starting at the same
> time. If you add a condition on the start_threshold you really don't
> need any hardware-driven start, do you?

What you've described is exactly what I had in mind, so we're still on 
the same page.

I wanted to make it clear that my implementation of start_at doesn't 
*prevent* client code starting on a threshold *and* using start_at, even 
if it seems to us like a strange idea.

Preventing the use of both requires us to show why it's never a useful 
idea, decide on policy (what do happens when client code tries to use 
both), and implement that policy. I'd rather just leave it as 'possible' :)

> Thanks,
> -Pierre
>
>
>

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

On 06/01/15 14:46, Pierre-Louis Bossart wrote:
>
>> I didn't make it clear that I reworked the patch so that it was
>> compatible with, but not dependent on, Pierre's work (viz: Timestamping
>> Evolutions). In particular you'll see that I lifted
>> snd_pcm_audio_tstamp_t from his patch: This means that our patches can
>> be discussed/merged independently. Was this a reasonable approach?
>
> Fine with me. There may be another dependency though, you probably need
> to provide INFO fields and the ability to query whether the hardware
> supports this functionality (which brings us back to the discussion on
> where to store those fields..)
>

Fair point :) But the patch, as it stands, doesn't depend on those fields.

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Raymond Yau]

>>
>>
>>>> Do your implementation need to set specific start threshold to prevent
>>>> the driver automatically start when you fill the buffer ?
>>>
>>>
>>>
>>>> Do the driver allow to start when there is no data ?
>>>>
>>>
>>> It's the responsibility of the client to set the start threshold to a
>>> safe and responsible value.
>>>
>>> It might suit some applications to allow both threshold start _and_
>>> start_at: My implementation doesn't preclude this.
>>
>>
>> Now I am confused... My understanding was that this feature is similar
>> to SSYNC in HDAudio, where everything is ready, buffers filled, DMAs
>> armed, FIFOs filled and the start condition only opens the last gate at
>> a specific time - possibly with multiple streams starting at the same
>> time. If you add a condition on the start_threshold you really don't
>> need any hardware-driven start, do you?
>
>
> What you've described is exactly what I had in mind, so we're still on
the same page.
>
> I wanted to make it clear that my implementation of start_at doesn't
*prevent* client code starting on a threshold *and* using start_at, even if
it seems to us like a strange idea.

http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html

Handshake between application and library

Do alsa lib assume all read/write must only call after calling
snd_pcm_sw_params() ?

It seem that proposed start_at() can only be call in SND_PCM_STATE_PREPARED
and should fail when stream is already running

Are there any mean to cancel the scheduled snd_pcm_start_at  ?

Seem there is no check when the application call snd_pcm_start_at()
multiple times

>
> Preventing the use of both requires us to show why it's never a useful
idea, decide on policy (what do happens when client code tries to use
both), and implement that policy. I'd rather just leave it as 'possible'
:at()

Re: [PATCH v2 1/1] alsa-lib: Add snd_pcm_start_at.

[Tim Cussins]

On 09/01/15 12:03, Raymond Yau wrote:
>
>  >>
>  >>
>  >>>> Do your implementation need to set specific start threshold to prevent
>  >>>> the driver automatically start when you fill the buffer ?
>  >>>
>  >>>
>  >>>
>  >>>> Do the driver allow to start when there is no data ?
>  >>>>
>  >>>
>  >>> It's the responsibility of the client to set the start threshold to a
>  >>> safe and responsible value.
>  >>>
>  >>> It might suit some applications to allow both threshold start _and_
>  >>> start_at: My implementation doesn't preclude this.
>  >>
>  >>
>  >> Now I am confused... My understanding was that this feature is similar
>  >> to SSYNC in HDAudio, where everything is ready, buffers filled, DMAs
>  >> armed, FIFOs filled and the start condition only opens the last gate at
>  >> a specific time - possibly with multiple streams starting at the same
>  >> time. If you add a condition on the start_threshold you really don't
>  >> need any hardware-driven start, do you?
>  >
>  >
>  > What you've described is exactly what I had in mind, so we're still
> on the same page.
>  >
>  > I wanted to make it clear that my implementation of start_at doesn't
> *prevent* client code starting on a threshold *and* using start_at, even
> if it seems to us like a strange idea.
>
> http://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html
>
> Handshake between application and library
>
> Do alsa lib assume all read/write must only call after calling
> snd_pcm_sw_params() ?
>
> It seem that proposed start_at() can only be call in
> SND_PCM_STATE_PREPARED and should fail when stream is already running

This sounds ok to me - I followed Nick Stoughton's lead on this.

> Are there any mean to cancel the scheduled snd_pcm_start_at  ?

There is no explicit mechanism in the proposed patch: The pending timer 
is cancelled if client code attempts to change the stream state. Would 
you like to see an explicit cancellation API?

> Seem there is no check when the application call snd_pcm_start_at()
> multiple times

The code only allows _one_ start_at timer to be pending. When 
snd_pcm_start_at() is called, the pending timer (if any) is cancelled 
(atomically) before being replaced by the new timer.

>  >
>  > Preventing the use of both requires us to show why it's never a
> useful idea, decide on policy (what do happens when client code tries to
> use both), and implement that policy. I'd rather just leave it as
> 'possible' :at()
>