[PATCH v2 00/10] audio timestamping evolutions

[PATCH v2 00/10] audio timestamping evolutions

[Pierre-Louis Bossart]

This series of patches was inspired by recent threads on the alsa
mailing list, as well issues detected with existing and upcoming
hardware:

1. there was a request from the PulseAudio community to get more
information from drivers to make rewinds safer. While the conclusion
was that it's nearly impossible for a driver developer to provide this
information, there are however ways to assess the granularity of the
hw_ptr updates using timestamping capabilities, and indirectly
understand how safe rewinds might be.

2. There was also a request to add a start_at capability based either
on system hr timers or a wall clock, which requires a means to expose
both types of information to applications. Rather than adding new sets
of timestamps, it is suggested the new start_at functionality relies
on the new definition provides by these patches

3. For new hardware, there is a neeed to let low-level drivers
handle timestamping instead of having the ALSA core do
it. Similarly there is a need to let the low-level driver update
the initial estimate for the trigger timestamp if there are
delays to start a stream (eg. with USB)

These patches try to provide an answer to these multiple needs by
building on the work done two years ago to expose wall clock
information to applications. The evolution is to let application
select which audio timestamp they are interested in, track the delay
and drifts between recurring measurements and get, when possible, an
idea of the accuracy of the underlying hardware. A backwards compatible mode
is provided in case userspace doesn't provide any timestamp selection (results
based on HDAudio wallclock for playback, hw_ptr in all other cases). 

The first 4 patches are corrections for misses in the way the system
and trigger timestamps are handled, and the last 6 provide the
additional audio timestamping selection. A second batch is planned to
enable hardware capabilities in a low-level drivers.

A corresponding set of patches is available for alsa-lib.

V2 changes:

trigger_tstamp:
trigger_tstamp_latched, pending redefined as bool
trigger_tstamp_latched reset in snd_pcm_pre_start()

audio_ts_config, report:
keep separate structure but use different bitfields for in and out.
use u32 instead of __u32, add comments that these structures are internal
COMPAT backwards compatible mode, uses WALL_CLOCK/LINK for HDAudio
playback and DEFAULT (hw_ptr) everywhere else

INFO bits:
reclaimed 32-bits from hw_params, renamed as info_ext
moved all timestamp info to info_ext

snd_pcm_status:
read only 32-bit audio_tstamp_data, ignore all other fields

Pierre-Louis Bossart (10):
  ALSA: core: don't override timestamp unconditionally
  ALSA: core: allow for trigger_tstamp snapshot in .trigger
  ALSA: hda: read trigger_timestamp immediately after starting DMA
  ALSA: usb: update trigger timestamp on first non-zero URB submitted
  ALSA: core: add info_ext field in hw_params and pcm_hardware
  ALSA: core: selection of audio_tstamp type and accuracy reports
  ALSA: core: pass audio tstamp config from userspace
  ALSA: core: pass audio tstamp config from userspace in compat mode
  ALSA: core: replace .wall_clock by .get_time_info
  ALSA: hda: replace .wallclock by .get_time_info

 Documentation/sound/alsa/timestamping.txt | 176 ++++++++++++++++++++++++++++++
 include/sound/pcm.h                       |  71 +++++++++++-
 include/uapi/sound/asound.h               |  31 +++++-
 sound/core/pcm_compat.c                   |  15 ++-
 sound/core/pcm_lib.c                      |  81 +++++++++-----
 sound/core/pcm_native.c                   |  41 ++++++-
 sound/pci/hda/hda_controller.c            |  44 ++++++--
 sound/usb/pcm.c                           |   9 ++
 8 files changed, 417 insertions(+), 51 deletions(-)
 create mode 100644 Documentation/sound/alsa/timestamping.txt

-- 
1.9.1

[PATCH v2 01/10] ALSA: core: don't override timestamp unconditionally

[Pierre-Louis Bossart]

timestamp in RUNNING mode is already taken in update_hw_ptr routine,
getting a new timestamp introduces offset between hw_ptr, audio_tstamp
and system time

Add else condition to read timestamp as fallback and only when
enabled

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/core/pcm_native.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 095d957..5dc83fb 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -719,8 +719,11 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 				runtime->status->audio_tstamp;
 			goto _tstamp_end;
 		}
+	} else {
+		/* get tstamp only in fallback mode and only if enabled */
+		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE)
+			snd_pcm_gettime(runtime, &status->tstamp);
 	}
-	snd_pcm_gettime(runtime, &status->tstamp);
  _tstamp_end:
 	status->appl_ptr = runtime->control->appl_ptr;
 	status->hw_ptr = runtime->status->hw_ptr;
-- 
1.9.1

[PATCH v2 02/10] ALSA: core: allow for trigger_tstamp snapshot in .trigger

[Pierre-Louis Bossart]

Don't use generic snapshot of trigger_tstamp if low-level driver or
hardware can get a more precise value for better audio/system time
synchronization.

Also add definitions for delayed updates if actual trigger tstamp
can be only be provided after a delay due to hardware constraints.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/pcm.h     | 2 ++
 sound/core/pcm_native.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 1e7f74a..2641d86 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -281,6 +281,8 @@ struct snd_pcm_runtime {
 	/* -- Status -- */
 	struct snd_pcm_substream *trigger_master;
 	struct timespec trigger_tstamp;	/* trigger timestamp */
+	bool trigger_tstamp_latched;     /* trigger timestamp latched in low-level driver/hardware */
+	bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
 	int overrange;
 	snd_pcm_uframes_t avail_max;
 	snd_pcm_uframes_t hw_ptr_base;	/* Position at buffer restart */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 5dc83fb..ae2a93a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -809,7 +809,8 @@ static void snd_pcm_trigger_tstamp(struct snd_pcm_substream *substream)
 	if (runtime->trigger_master == NULL)
 		return;
 	if (runtime->trigger_master == substream) {
-		snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
+		if (runtime->trigger_tstamp_latched == 0)
+			snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
 	} else {
 		snd_pcm_trigger_tstamp(runtime->trigger_master);
 		runtime->trigger_tstamp = runtime->trigger_master->runtime->trigger_tstamp;
@@ -978,6 +979,7 @@ static int snd_pcm_pre_start(struct snd_pcm_substream *substream, int state)
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    !snd_pcm_playback_data(substream))
 		return -EPIPE;
+	runtime->trigger_tstamp_latched = 0;
 	runtime->trigger_master = substream;
 	return 0;
 }
-- 
1.9.1

[PATCH v2 03/10] ALSA: hda: read trigger_timestamp immediately after starting DMA

[Pierre-Louis Bossart]

Make sure wallclock counter and trigger timestamp are read very
close to each other for better alignment.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/pci/hda/hda_controller.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 8337645..9ad9454 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -657,6 +657,9 @@ static int azx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 		azx_writel(chip, SSYNC, azx_readl(chip, SSYNC) & ~sbits);
 	if (start) {
 		azx_timecounter_init(substream, 0, 0);
+		snd_pcm_gettime(substream->runtime, &substream->runtime->trigger_tstamp);
+		substream->runtime->trigger_tstamp_latched = 1;
+
 		if (nsync > 1) {
 			cycle_t cycle_last;
 
-- 
1.9.1

[PATCH v2 04/10] ALSA: usb: update trigger timestamp on first non-zero URB submitted

[Pierre-Louis Bossart]

The first URBs are submitted during the prepare stage. When .trigger is
called, the ALSA core saves a trigger tstamp that doesn't correspond to
the actual time when the samples are submitted. The trigger_tstamp is
now updated when the first data are submitted to avoid any time offsets.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/usb/pcm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 0d8aba5..7c36fc1 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1464,6 +1464,14 @@ static void prepare_playback_urb(struct snd_usb_substream *subs,
 	subs->last_frame_number = usb_get_current_frame_number(subs->dev);
 	subs->last_frame_number &= 0xFF; /* keep 8 LSBs */
 
+	if (runtime->trigger_tstamp_pending_update == 1) {
+		/* this is the first actual URB submitted,
+		 * update trigger timestamp to reflect actual start time
+		 */
+		snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
+		runtime->trigger_tstamp_pending_update = 0;
+	}
+
 	spin_unlock_irqrestore(&subs->lock, flags);
 	urb->transfer_buffer_length = bytes;
 	if (period_elapsed)
@@ -1550,6 +1558,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
+		substream->runtime->trigger_tstamp_pending_update = 1;
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		subs->data_endpoint->prepare_data_urb = prepare_playback_urb;
 		subs->data_endpoint->retire_data_urb = retire_playback_urb;
-- 
1.9.1

[PATCH v2 05/10] ALSA: core: add info_ext field in hw_params and pcm_hardware

[Pierre-Louis Bossart]

reclaim 4 bytes from reserved bytes to store new info flags related to
hardware setup and capabilities.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/pcm.h         | 1 +
 include/uapi/sound/asound.h | 3 ++-
 sound/core/pcm_compat.c     | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 2641d86..05df271 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -44,6 +44,7 @@
 
 struct snd_pcm_hardware {
 	unsigned int info;		/* SNDRV_PCM_INFO_* */
+	unsigned int info_ext;		/* SNDRV_PCM_INFO_EXT* */
 	u64 formats;			/* SNDRV_PCM_FMTBIT_* */
 	unsigned int rates;		/* SNDRV_PCM_RATE_* */
 	unsigned int rate_min;		/* min rate */
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 1f23cd6..43e26af 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -375,7 +375,8 @@ struct snd_pcm_hw_params {
 	unsigned int rate_num;		/* R: rate numerator */
 	unsigned int rate_den;		/* R: rate denominator */
 	snd_pcm_uframes_t fifo_size;	/* R: chip FIFO size in frames */
-	unsigned char reserved[64];	/* reserved for future */
+	unsigned int info_ext;          /* R: additional Info flags for setup */
+	unsigned char reserved[60];	/* reserved for future */
 };
 
 enum {
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index 2d957ba..d7c5669 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -87,7 +87,8 @@ struct snd_pcm_hw_params32 {
 	u32 rate_num;
 	u32 rate_den;
 	u32 fifo_size;
-	unsigned char reserved[64];
+	u32 info_ext;
+	unsigned char reserved[60];
 };
 
 struct snd_pcm_sw_params32 {
-- 
1.9.1

[PATCH v2 06/10] ALSA: core: selection of audio_tstamp type and accuracy reports

[Pierre-Louis Bossart]

Audio timestamps can be extracted from sample counters, wall clocks,
PHC clocks (Ethernet AVB), on-demand synchronized snapshots. This
patch provides the ability to report timestamping capabilities, select
timestamp types and retrieve timestamp accuracy, if supported.

This functionality is introduced by reclaiming the reserved_aligned
field introduced by commit9c7066aef4a5eb8e4063de28f06c508bf6f2963a
in snd_pcm_status to provide userspace with selection/query capabilities.

snd_pcm_mmap_status remains a read-only structure with only
the audio timestamp value accessible from user space. The selection
of audio timestamp type is done through snd_pcm_status only

This commit does not impact ABI and does not impact the default
behavior. By default audio timestamp is aligned with hw_pointer and
reports the DMA position. Backwards compatibility is handled by using
the HDAudio wall clock for playback and the hw_ptr for all other
cases.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 Documentation/sound/alsa/timestamping.txt | 176 ++++++++++++++++++++++++++++++
 include/sound/pcm.h                       |  62 +++++++++++
 include/uapi/sound/asound.h               |  28 ++++-
 3 files changed, 263 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/sound/alsa/timestamping.txt

diff --git a/Documentation/sound/alsa/timestamping.txt b/Documentation/sound/alsa/timestamping.txt
new file mode 100644
index 0000000..3454449
--- /dev/null
+++ b/Documentation/sound/alsa/timestamping.txt
@@ -0,0 +1,176 @@
+The ALSA API can provide two different system timestamps:
+
+- Trigger_tstamp is the system time snapshot taken when the .trigger
+callback is invoked. This snapshot is taken by the ALSA core in the
+general case, but specific hardware may have synchronization
+capabilities or conversely may only be able to provide a correct
+estimate with a delay. In the latter two cases, the low-level driver
+is responsible for updating the trigger_tstamp at the most appropriate
+and precise moment. Applications should not rely solely on the first
+trigger_tstamp but update their internal calculations if the driver
+provides a refined estimate with a delay.
+
+- tstamp is the current system timestamp updated during the last
+event or application query.
+The difference (tstamp - trigger_tstamp) defines the elapsed time.
+
+The ALSA API provides reports two basic pieces of information, avail
+and delay, which combined with the trigger and current system
+timestamps allow for applications to keep track of the 'fullness' of
+the ring buffer and the amount of queued samples.
+
+The use of these different pointers and time information depends on
+the application needs:
+
+- 'avail' reports how much can be written in the ring buffer
+- 'delay' reports the time it will take to hear a new sample after all
+queued samples have been played out.
+
+When timestamps are enabled, the avail/delay information is reported
+along with a snapshot of system time. Applications can select from
+CLOCK_REALTIME (NTP corrections including going backwards),
+CLOCK_MONOTONIC (NTP corrections but never going backwards),
+CLOCK_MONOTIC_RAW (without NTP corrections) and change the mode
+dynamically with sw_params
+
+
+The ALSA API also provide an audio_tstamp which reflects the passage
+of time as measured by different components of audio hardware.  In
+ascii-art, this could be represented as follows (for the playback
+case):
+
+
+--------------------------------------------------------------> time
+  ^               ^              ^                ^           ^
+  |               |              |                |           |
+ analog         link            dma              app       FullBuffer
+ time           time           time              time        time
+  |               |              |                |           |
+  |< codec delay >|<--hw delay-->|<queued samples>|<---avail->|
+  |<----------------- delay---------------------->|           |
+			         |<----ring buffer length---->|
+
+The analog time is taken at the last stage of the playback, as close
+as possible to the actual transducer
+
+The link time is taken at the output of the SOC/chipset as the samples
+are pushed on a link. The link time can be directly measured if
+supported in hardware by sample counters or wallclocks (e.g. with
+HDAudio 24MHz or PTP clock for networked solutions) or indirectly
+estimated (e.g. with the frame counter in USB).
+
+The DMA time is measured using counters - typically the least reliable
+of all measurements due to the bursty natured of DMA transfers.
+
+The app time corresponds to the time tracked by an application after
+writing in the ring buffer.
+
+The application can query what the hardware supports, define which
+audio time it wants reported by selecting the relevant settings in
+audio_tstamp_config fields, get an estimate of the timestamp
+accuracy. It can also request the delay-to-analog be included in the
+measurement. Direct access to the link time is very interesting on
+platforms that provide an embedded DSP; measuring directly the link
+time with dedicated hardware, possibly synchronized with system time,
+removes the need to keep track of internal DSP processing times and
+latency.
+
+In case the application requests an audio tstamp that is not supported
+in hardware/low-level driver, the type is overriden as DEFAULT and the
+timestamp will report the DMA time based on the hw_pointer value.
+
+For backwards compatibility with previous implementations that did not
+provide timestamp selection, with a zero-valued COMPAT timestamp type
+the results will default to the HDAudio wall clock for playback
+streams and to the DMA time (hw_ptr) in all other cases.
+
+The audio timestamp accuracy can be returned to user-space, so that
+appropriate decisions are made:
+
+- for dma time (default), the granularity of the transfers can be
+  inferred from the steps between updates and in turn provide
+  information on how much the application pointer can be rewound
+  safely.
+
+- the link time can be used to track long-term drifts between audio
+  and system time using the (tstamp-trigger_tstamp)/audio_tstamp
+  ratio, the precision helps define how much smoothing/low-pass
+  filtering is required. The link time can be either reset on startup
+  or reported as is (the latter being useful to compare progress of
+  different streams - but may require the wallclock to be always
+  running and not wrap-around during idle periods). If supported in
+  hardware, the absolute link time could also be used to define a
+  precise start time (patches WIP)
+
+- including the delay in the audio timestamp may
+  counter-intuitively not increase the precision of timestamps, e.g. if a
+  codec includes variable-latency DSP processing or a chain of
+  hardware components the delay is typically not known with precision.
+
+The accuracy is reported with a mantissa and base10 exponent to cover
+the wide range of precision from 10s of ns to 10s of ms. The exponent is set
+to zero for ns, 3 for us, 6 for ms, 9 for s.
+
+Due to the varied nature of timestamping needs, even for a single
+application, the audio_tstamp_config can be changed dynamically.
+
+Examples of typestamping with HDaudio:
+
+1. DMA timestamp, no compensation for DMA+analog delay
+$ ./audio_time  -p --ts_type=1
+playback: systime: 341121338 nsec, audio time 342000000 nsec, 	systime delta -878662
+playback: systime: 426236663 nsec, audio time 427187500 nsec, 	systime delta -950837
+playback: systime: 597080580 nsec, audio time 598000000 nsec, 	systime delta -919420
+playback: systime: 682059782 nsec, audio time 683020833 nsec, 	systime delta -961051
+playback: systime: 852896415 nsec, audio time 853854166 nsec, 	systime delta -957751
+playback: systime: 937903344 nsec, audio time 938854166 nsec, 	systime delta -950822
+
+2. DMA timestamp, compensation for DMA+analog delay
+$ ./audio_time  -p --ts_type=1 -d
+playback: systime: 341053347 nsec, audio time 341062500 nsec, 	systime delta -9153
+playback: systime: 426072447 nsec, audio time 426062500 nsec, 	systime delta 9947
+playback: systime: 596899518 nsec, audio time 596895833 nsec, 	systime delta 3685
+playback: systime: 681915317 nsec, audio time 681916666 nsec, 	systime delta -1349
+playback: systime: 852741306 nsec, audio time 852750000 nsec, 	systime delta -8694
+
+3. link timestamp, compensation for DMA+analog delay
+$ ./audio_time  -p --ts_type=2 -d
+playback: systime: 341060004 nsec, audio time 341062791 nsec, 	systime delta -2787
+playback: systime: 426242074 nsec, audio time 426244875 nsec, 	systime delta -2801
+playback: systime: 597080992 nsec, audio time 597084583 nsec, 	systime delta -3591
+playback: systime: 682084512 nsec, audio time 682088291 nsec, 	systime delta -3779
+playback: systime: 852936229 nsec, audio time 852940916 nsec, 	systime delta -4687
+playback: systime: 938107562 nsec, audio time 938112708 nsec, 	systime delta -5146
+
+Example 1 shows that the timestamp at the DMA level is close to 1ms
+ahead of the actual playback time (as a side time this sort of
+measurement can help define rewind safeguards). Compensating for the
+DMA-link delay in example 2 helps remove the hardware buffering abut
+the information is still very jittery, with up to one sample of
+error. In example 3 where the timestamps are measured with the link
+wallclock, the timestamps show a monotonic behavior and a lower
+dispersion.
+
+Example 3 and 4 are with USB audio class. Example 3 shows a high
+offset between audio time and system time due to buffering. Example 4
+shows how compensating for the delay exposes a 1ms accuracy (due to
+the use of the frame counter by the driver)
+
+Example 3: DMA timestamp, no compensation for delay, delta of ~5ms
+$ ./audio_time -p -Dhw:1 -t1
+playback: systime: 120174019 nsec, audio time 125000000 nsec, 	systime delta -4825981
+playback: systime: 245041136 nsec, audio time 250000000 nsec, 	systime delta -4958864
+playback: systime: 370106088 nsec, audio time 375000000 nsec, 	systime delta -4893912
+playback: systime: 495040065 nsec, audio time 500000000 nsec, 	systime delta -4959935
+playback: systime: 620038179 nsec, audio time 625000000 nsec, 	systime delta -4961821
+playback: systime: 745087741 nsec, audio time 750000000 nsec, 	systime delta -4912259
+playback: systime: 870037336 nsec, audio time 875000000 nsec, 	systime delta -4962664
+
+Example 4: DMA timestamp, compensation for delay, delay of ~1ms
+$ ./audio_time -p -Dhw:1 -t1 -d
+playback: systime: 120190520 nsec, audio time 120000000 nsec, 	systime delta 190520
+playback: systime: 245036740 nsec, audio time 244000000 nsec, 	systime delta 1036740
+playback: systime: 370034081 nsec, audio time 369000000 nsec, 	systime delta 1034081
+playback: systime: 495159907 nsec, audio time 494000000 nsec, 	systime delta 1159907
+playback: systime: 620098824 nsec, audio time 619000000 nsec, 	systime delta 1098824
+playback: systime: 745031847 nsec, audio time 744000000 nsec, 	systime delta 1031847
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 05df271..77f11fe 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -61,6 +61,9 @@ struct snd_pcm_hardware {
 
 struct snd_pcm_substream;
 
+struct snd_pcm_audio_tstamp_config; /* definitions further down */
+struct snd_pcm_audio_tstamp_report;
+
 struct snd_pcm_ops {
 	int (*open)(struct snd_pcm_substream *substream);
 	int (*close)(struct snd_pcm_substream *substream);
@@ -278,6 +281,61 @@ struct snd_pcm_hw_constraint_list {
 
 struct snd_pcm_hwptr_log;
 
+/*
+ * userspace-provided audio timestamp config to kernel,
+ * structure is for internal use only and filled with dedicated unpack routine
+ */
+struct snd_pcm_audio_tstamp_config {
+	/* 5 of max 8 bits used */
+	u32 type_requested:4;
+	u32 report_delay:1; /* add total delay to A/D or D/A */
+};
+
+static inline void snd_pcm_unpack_audio_tstamp_config(__u32 data,
+						struct snd_pcm_audio_tstamp_config *config)
+{
+	config->type_requested = data & 0xFF;
+	config->report_delay = (data >> 4) & 1;
+}
+
+/*
+ * kernel-provided audio timestamp report to user-space
+ * structure is for internal use only and read by dedicated pack routine
+ */
+struct snd_pcm_audio_tstamp_report {
+	/* 17 of 24 bits max used*/
+
+	/* for backwards compatibility */
+	u32 valid:1;
+
+	/* actual type if hardware could not support requested timestamp */
+	u32 actual_type:4;
+
+	/* accuracy represented in mantissa/exponent form */
+	u32 accuracy_report:1; /* 0 if accuracy unknown, 1 if rest of structure is valid */
+	u32 accuracy_m:7; /* 0..127, ~3 significant digit for mantissa */
+	u32 accuracy_e:4; /* base10 exponent, 0 for ns, 3 for us, 6 for ms, 9 for s */
+};
+
+static inline void snd_pcm_pack_audio_tstamp_report(__u32 *data,
+						struct snd_pcm_audio_tstamp_report *report)
+{
+	u32 tmp;
+
+	tmp = report->accuracy_e;
+	tmp <<= 7;
+	tmp |= report->accuracy_m;
+	tmp <<= 1;
+	tmp |= report->accuracy_report;
+	tmp <<= 4;
+	tmp |= report->actual_type;
+	tmp <<= 1;
+	tmp |= report->valid;
+
+	*data |= (tmp << 8);
+}
+
+
 struct snd_pcm_runtime {
 	/* -- Status -- */
 	struct snd_pcm_substream *trigger_master;
@@ -359,6 +417,10 @@ struct snd_pcm_runtime {
 
 	struct snd_dma_buffer *dma_buffer_p;	/* allocated buffer */
 
+	/* -- audio timestamp config -- */
+	struct snd_pcm_audio_tstamp_config audio_tstamp_config;
+	struct snd_pcm_audio_tstamp_report audio_tstamp_report;
+
 #if defined(CONFIG_SND_PCM_OSS) || defined(CONFIG_SND_PCM_OSS_MODULE)
 	/* -- OSS things -- */
 	struct snd_pcm_oss_runtime oss;
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 43e26af..333952a 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -267,9 +267,15 @@ typedef int __bitwise snd_pcm_subformat_t;
 #define SNDRV_PCM_INFO_JOINT_DUPLEX	0x00200000	/* playback and capture stream are somewhat correlated */
 #define SNDRV_PCM_INFO_SYNC_START	0x00400000	/* pcm support some kind of sync go */
 #define SNDRV_PCM_INFO_NO_PERIOD_WAKEUP	0x00800000	/* period wakeup can be disabled */
-#define SNDRV_PCM_INFO_HAS_WALL_CLOCK   0x01000000      /* has audio wall clock for audio/system time sync */
+#define SNDRV_PCM_INFO_HAS_WALL_CLOCK   0x01000000      /* deprecated, use LINK_ATIME */
 #define SNDRV_PCM_INFO_FIFO_IN_FRAMES	0x80000000	/* internal kernel flag - FIFO size is in frames */
 
+#define SNDRV_PCM_INFO_EXT_HAS_LINK_ATIME              0x00000001  /* report hardware link audio time, reset on startup */
+#define SNDRV_PCM_INFO_EXT_HAS_LINK_ABSOLUTE_ATIME     0x00000002  /* report absolute hardware link audio time, not reset on startup */
+#define SNDRV_PCM_INFO_EXT_HAS_LINK_ESTIMATED_ATIME    0x00000004  /* report estimated link audio time */
+#define SNDRV_PCM_INFO_EXT_HAS_LINK_SYNCHRONIZED_ATIME 0x00000008  /* report synchronized audio/system time */
+
+
 typedef int __bitwise snd_pcm_state_t;
 #define	SNDRV_PCM_STATE_OPEN		((__force snd_pcm_state_t) 0) /* stream is open */
 #define	SNDRV_PCM_STATE_SETUP		((__force snd_pcm_state_t) 1) /* stream has a setup */
@@ -408,6 +414,22 @@ struct snd_pcm_channel_info {
 	unsigned int step;		/* samples distance in bits */
 };
 
+enum {
+	/*
+	 *  first definition for backwards compatibility only,
+	 *  maps to wallclock/link time for HDAudio playback and DEFAULT/DMA time for everything else
+	 */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_COMPAT = 0,
+
+	/* timestamp definitions */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT = 1,           /* DMA time, reported as per hw_ptr */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK = 2,	           /* link time reported by sample or wallclock counter, reset on startup */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_ABSOLUTE = 3,	   /* link time reported by sample or wallclock counter, not reset on startup */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_ESTIMATED = 4,    /* link time estimated indirectly */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED = 5, /* link time synchronized with system time */
+	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LAST = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED
+};
+
 struct snd_pcm_status {
 	snd_pcm_state_t state;		/* stream state */
 	struct timespec trigger_tstamp;	/* time when stream was started/stopped/paused */
@@ -419,8 +441,8 @@ struct snd_pcm_status {
 	snd_pcm_uframes_t avail_max;	/* max frames available on hw since last status */
 	snd_pcm_uframes_t overrange;	/* count of ADC (capture) overrange detections from last status */
 	snd_pcm_state_t suspended_state; /* suspended stream state */
-	__u32 reserved_alignment;	/* must be filled with zero */
-	struct timespec audio_tstamp;	/* from sample counter or wall clock */
+	__u32 audio_tstamp_data;	 /* needed for 64-bit alignment, used for configs/report to/from userspace */
+	struct timespec audio_tstamp;	/* sample counter, wall clock, PHC or on-demand sync'ed */
 	unsigned char reserved[56-sizeof(struct timespec)]; /* must be filled with zero */
 };
 
-- 
1.9.1

[PATCH v2 07/10] ALSA: core: pass audio tstamp config from userspace

[Pierre-Louis Bossart]

Let userspace select audio timestamp config, ignore and zero all
other fields

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/core/pcm_native.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index ae2a93a..8f8aab0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -756,8 +756,16 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_status status;
 	int res;
-	
+	u32  audio_tstamp_data;
+	u32  __user *_audio_tstamp_data;
+
+	/* get audio_tstamp_data from user, ignore rest of status structure */
+	_audio_tstamp_data = (u32 __user *)(&_status->audio_tstamp_data);
+	if (get_user(audio_tstamp_data, _audio_tstamp_data))
+		return -EFAULT;
 	memset(&status, 0, sizeof(status));
+	status.audio_tstamp_data = audio_tstamp_data;
+
 	res = snd_pcm_status(substream, &status);
 	if (res < 0)
 		return res;
-- 
1.9.1

[PATCH v2 08/10] ALSA: core: pass audio tstamp config from userspace in compat mode

[Pierre-Louis Bossart]

Let userspace select audio timestamp config, ignore and zero all
other fields
Use audio_tstamp_data to retrieve config and pass report back to
user space

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/core/pcm_compat.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index d7c5669..65e0310 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -195,7 +195,7 @@ struct snd_pcm_status32 {
 	u32 avail_max;
 	u32 overrange;
 	s32 suspended_state;
-	u32 reserved_alignment;
+	u32 audio_tstamp_data;
 	struct compat_timespec audio_tstamp;
 	unsigned char reserved[56-sizeof(struct compat_timespec)];
 } __attribute__((packed));
@@ -206,6 +206,15 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
 {
 	struct snd_pcm_status status;
 	int err;
+	u32  audio_tstamp_data;
+	u32  __user *_audio_tstamp_data;
+
+	/* get audio_tstamp_data from user, ignore rest of status structure */
+	_audio_tstamp_data = (u32 __user *)(&src->audio_tstamp_data);
+	if (get_user(audio_tstamp_data, _audio_tstamp_data))
+		return -EFAULT;
+	memset(&status, 0, sizeof(status));
+	status.audio_tstamp_data = audio_tstamp_data;
 
 	err = snd_pcm_status(substream, &status);
 	if (err < 0)
@@ -223,6 +232,7 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
 	    put_user(status.avail_max, &src->avail_max) ||
 	    put_user(status.overrange, &src->overrange) ||
 	    put_user(status.suspended_state, &src->suspended_state) ||
+	    put_user(status.audio_tstamp_data, &src->audio_tstamp_data) ||
 	    compat_put_timespec(&status.audio_tstamp, &src->audio_tstamp))
 		return -EFAULT;
 
-- 
1.9.1

[PATCH v2 09/10] ALSA: core: replace .wall_clock by .get_time_info

[Pierre-Louis Bossart]

Introduce more generic .get_time_info to retrieve
system timestamp and audio timestamp in single routine.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/pcm.h     |  6 ++--
 sound/core/pcm_lib.c    | 81 +++++++++++++++++++++++++++++++------------------
 sound/core/pcm_native.c | 22 ++++++++++++++
 3 files changed, 78 insertions(+), 31 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 77f11fe..7118655 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -75,8 +75,10 @@ struct snd_pcm_ops {
 	int (*prepare)(struct snd_pcm_substream *substream);
 	int (*trigger)(struct snd_pcm_substream *substream, int cmd);
 	snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *substream);
-	int (*wall_clock)(struct snd_pcm_substream *substream,
-			  struct timespec *audio_ts);
+	int (*get_time_info)(struct snd_pcm_substream *substream,
+			struct timespec *system_ts, struct timespec *audio_ts,
+			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
+			struct snd_pcm_audio_tstamp_report *audio_tstamp_report);
 	int (*copy)(struct snd_pcm_substream *substream, int channel,
 		    snd_pcm_uframes_t pos,
 		    void __user *buf, snd_pcm_uframes_t count);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index ec9e786..6c61e6c 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -232,6 +232,42 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static void update_audio_tstamp(struct snd_pcm_substream *substream,
+				struct timespec *curr_tstamp,
+				struct timespec *audio_tstamp)
+
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	u64 audio_frames, audio_nsecs;
+
+	if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
+		runtime->status->tstamp = *curr_tstamp;
+
+		if (!(substream->ops->get_time_info) ||
+			(runtime->audio_tstamp_report.actual_type == SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)) {
+
+			/*
+			 * provide audio timestamp derived from pointer position
+			 * add delay only if requested
+			 */
+
+			audio_frames = runtime->hw_ptr_wrap
+				+ runtime->status->hw_ptr;
+
+			if (runtime->audio_tstamp_config.report_delay) {
+				if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+					audio_frames -=  runtime->delay;
+				else
+					audio_frames +=  runtime->delay;
+			}
+			audio_nsecs = div_u64(audio_frames * 1000000000LL,
+					runtime->rate);
+			*audio_tstamp = ns_to_timespec(audio_nsecs);
+		}
+		runtime->status->audio_tstamp = *audio_tstamp;
+	}
+}
+
 static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 				  unsigned int in_interrupt)
 {
@@ -256,11 +292,18 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 	pos = substream->ops->pointer(substream);
 	curr_jiffies = jiffies;
 	if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
-		snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp);
-
-		if ((runtime->hw.info & SNDRV_PCM_INFO_HAS_WALL_CLOCK) &&
-			(substream->ops->wall_clock))
-			substream->ops->wall_clock(substream, &audio_tstamp);
+		if ((substream->ops->get_time_info) &&
+			(runtime->audio_tstamp_config.type_requested != SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)) {
+			substream->ops->get_time_info(substream, &curr_tstamp,
+						&audio_tstamp,
+						&runtime->audio_tstamp_config,
+						&runtime->audio_tstamp_report);
+
+			/* re-test in case tstamp type is not supported in hardware and was demoted to DEFAULT */
+			if (runtime->audio_tstamp_report.actual_type == SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)
+				snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp);
+		} else
+			snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp);
 	}
 
 	if (pos == SNDRV_PCM_POS_XRUN) {
@@ -403,8 +446,10 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 	}
 
  no_delta_check:
-	if (runtime->status->hw_ptr == new_hw_ptr)
+	if (runtime->status->hw_ptr == new_hw_ptr) {
+		update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
 		return 0;
+	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    runtime->silence_size > 0)
@@ -426,30 +471,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 		snd_BUG_ON(crossed_boundary != 1);
 		runtime->hw_ptr_wrap += runtime->boundary;
 	}
-	if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
-		runtime->status->tstamp = curr_tstamp;
 
-		if (!(runtime->hw.info & SNDRV_PCM_INFO_HAS_WALL_CLOCK)) {
-			/*
-			 * no wall clock available, provide audio timestamp
-			 * derived from pointer position+delay
-			 */
-			u64 audio_frames, audio_nsecs;
-
-			if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-				audio_frames = runtime->hw_ptr_wrap
-					+ runtime->status->hw_ptr
-					- runtime->delay;
-			else
-				audio_frames = runtime->hw_ptr_wrap
-					+ runtime->status->hw_ptr
-					+ runtime->delay;
-			audio_nsecs = div_u64(audio_frames * 1000000000LL,
-					runtime->rate);
-			audio_tstamp = ns_to_timespec(audio_nsecs);
-		}
-		runtime->status->audio_tstamp = audio_tstamp;
-	}
+	update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
 
 	return snd_pcm_update_state(substream, runtime);
 }
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8f8aab0..8a84da7 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -706,6 +706,23 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	snd_pcm_stream_lock_irq(substream);
+
+	snd_pcm_unpack_audio_tstamp_config(status->audio_tstamp_data,
+					&runtime->audio_tstamp_config);
+
+	/* backwards compatible behavior */
+	if (runtime->audio_tstamp_config.type_requested ==
+		SNDRV_PCM_AUDIO_TSTAMP_TYPE_COMPAT) {
+		if (runtime->hw.info & SNDRV_PCM_INFO_HAS_WALL_CLOCK)
+			runtime->audio_tstamp_config.type_requested =
+				SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
+		else
+			runtime->audio_tstamp_config.type_requested =
+				SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT;
+		runtime->audio_tstamp_report.valid = 0;
+	} else
+		runtime->audio_tstamp_report.valid = 1;
+
 	status->state = runtime->status->state;
 	status->suspended_state = runtime->status->suspended_state;
 	if (status->state == SNDRV_PCM_STATE_OPEN)
@@ -717,6 +734,11 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 			status->tstamp = runtime->status->tstamp;
 			status->audio_tstamp =
 				runtime->status->audio_tstamp;
+			if (runtime->audio_tstamp_report.valid == 1)
+				/* backwards compatibility, no report provided in COMPAT mode */
+				snd_pcm_pack_audio_tstamp_report(&status->audio_tstamp_data,
+								&runtime->audio_tstamp_report);
+
 			goto _tstamp_end;
 		}
 	} else {
-- 
1.9.1

[PATCH v2 10/10] ALSA: hda: replace .wallclock by .get_time_info

[Pierre-Louis Bossart]

No real functional change, only take wall clock and system time
in same routine and add accuracy report.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/pci/hda/hda_controller.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 9ad9454..53b29ea 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -732,17 +732,33 @@ static snd_pcm_uframes_t azx_pcm_pointer(struct snd_pcm_substream *substream)
 			       azx_get_position(chip, azx_dev));
 }
 
-static int azx_get_wallclock_tstamp(struct snd_pcm_substream *substream,
-				struct timespec *ts)
+static int azx_get_time_info(struct snd_pcm_substream *substream,
+			struct timespec *system_ts, struct timespec *audio_ts,
+			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
+			struct snd_pcm_audio_tstamp_report *audio_tstamp_report)
 {
 	struct azx_dev *azx_dev = get_azx_dev(substream);
 	u64 nsec;
 
-	nsec = timecounter_read(&azx_dev->azx_tc);
-	nsec = div_u64(nsec, 3); /* can be optimized */
-	nsec = azx_adjust_codec_delay(substream, nsec);
+	if ((substream->runtime->hw.info_ext & SNDRV_PCM_INFO_EXT_HAS_LINK_ATIME) &&
+		(audio_tstamp_config->type_requested == SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK)) {
 
-	*ts = ns_to_timespec(nsec);
+		snd_pcm_gettime(substream->runtime, system_ts);
+
+		nsec = timecounter_read(&azx_dev->azx_tc);
+		nsec = div_u64(nsec, 3); /* can be optimized */
+		if (audio_tstamp_config->report_delay)
+			nsec = azx_adjust_codec_delay(substream, nsec);
+
+		*audio_ts = ns_to_timespec(nsec);
+
+		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
+		audio_tstamp_report->accuracy_report = 1; /* rest of structure is valid */
+		audio_tstamp_report->accuracy_m = 42; /* 24 MHz WallClock == 42ns resolution */
+		audio_tstamp_report->accuracy_e = 0;
+
+	} else
+		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT;
 
 	return 0;
 }
@@ -756,8 +772,9 @@ static struct snd_pcm_hardware azx_pcm_hw = {
 				 /* SNDRV_PCM_INFO_RESUME |*/
 				 SNDRV_PCM_INFO_PAUSE |
 				 SNDRV_PCM_INFO_SYNC_START |
-				 SNDRV_PCM_INFO_HAS_WALL_CLOCK |
+				 SNDRV_PCM_INFO_HAS_WALL_CLOCK | /* legacy */
 				 SNDRV_PCM_INFO_NO_PERIOD_WAKEUP),
+	.info_ext =             (SNDRV_PCM_INFO_EXT_HAS_LINK_ATIME),
 	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
 	.rates =		SNDRV_PCM_RATE_48000,
 	.rate_min =		48000,
@@ -842,10 +859,12 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 		return -EINVAL;
 	}
 
-	/* disable WALLCLOCK timestamps for capture streams
+	/* disable LINK_ATIME timestamps for capture streams
 	   until we figure out how to handle digital inputs */
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-		runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_WALL_CLOCK;
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_WALL_CLOCK; /* legacy */
+		runtime->hw.info_ext &= ~SNDRV_PCM_INFO_EXT_HAS_LINK_ATIME;
+	}
 
 	spin_lock_irqsave(&chip->reg_lock, flags);
 	azx_dev->substream = substream;
@@ -877,7 +896,7 @@ static struct snd_pcm_ops azx_pcm_ops = {
 	.prepare = azx_pcm_prepare,
 	.trigger = azx_pcm_trigger,
 	.pointer = azx_pcm_pointer,
-	.wall_clock =  azx_get_wallclock_tstamp,
+	.get_time_info =  azx_get_time_info,
 	.mmap = azx_pcm_mmap,
 	.page = snd_pcm_sgbuf_ops_page,
 };
-- 
1.9.1

Re: [PATCH v2 00/10] audio timestamping evolutions

[Jaroslav Kysela]

Dne 19.12.2014 v 18:25 Pierre-Louis Bossart napsal(a):
> This series of patches was inspired by recent threads on the alsa
> mailing list, as well issues detected with existing and upcoming
> hardware:

I don't like some aspects in the interface:

1) ext_info member is not required - the standard info field
   has enough free bits
2) the whole struct snd_pcm_status is R/O -
   _IOR('A', 0x20, struct snd_pcm_status); I believe that it's much
   better to add new audio_tstamp_type to sw_params, but see (4)
3) accuracy - I would use uint64_t and report accuracy in pico-seconds
   (range from 0 picoseconds to 18446744 seconds); yes, use next bytes
   from the reserved part of status struct. the __u32 might be used only
   for flags
4) if there is a motivation to call / obtain timestamps for multiple
   purposes (audio tstamp types), then offer to return all these
   timestamps in one shot rather than do multiple queries (again, use
   reserved bytes)

			Thanks,
				Jaroslav

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

Re: [PATCH v2 00/10] audio timestamping evolutions

[Pierre-Louis Bossart]

Thanks for the review Jaroslav

> 1) ext_info member is not required - the standard info field
>     has enough free bits

Well this was added at Takashi's request, the initial patches didn't 
rely on this extension...I can roll back those changes if this is the 
consensus.

> 2) the whole struct snd_pcm_status is R/O -
>     _IOR('A', 0x20, struct snd_pcm_status); I believe that it's much
>     better to add new audio_tstamp_type to sw_params, but see (4)

I thought about this, but
- selecting the timestamp type with sw_params would require multiple 
system calls to achieve the same results. Every additional call or delay 
changes the accuracy of the results and correlation between data 
consumption and timing reports.
- existing code already relies on snd_pcm_status to retrieve system and 
audio timestamps, the selection through other means would make the code 
more complicated.

> 3) accuracy - I would use uint64_t and report accuracy in pico-seconds
>     (range from 0 picoseconds to 18446744 seconds); yes, use next bytes
>     from the reserved part of status struct. the __u32 might be used only
>     for flags

The timestamps are not better than nanoseconds. I don't actually know of 
any link that uses a wallclock higher than 24/48Mhz, so that's already 
~20-40ns already. It seemed overkill to me do use more than 3 
significant digits and an exponent to represent a nominal value that 
doesn't take jitter and drift into account anyway. The idea was to 
provide a qualitative value, not an actual measurement.

> 4) if there is a motivation to call / obtain timestamps for multiple
>     purposes (audio tstamp types), then offer to return all these
>     timestamps in one shot rather than do multiple queries (again, use
>     reserved bytes)

I thought about this case but I couldn't find any practical uses of 
multiple timestamps at the same time. In the absence of any atomic 
hardware snapshots of multiple counters, reading multiple values 
sequentially from different counters would actually water-down the 
accuracy and value of the timestamps returned. It's already hard-enough 
to track a single pair of audio and system counters.

Re: [PATCH v2 10/10] ALSA: hda: replace .wallclock by .get_time_info

[Raymond Yau]

>
> No real functional change, only take wall clock and system time
> in same routine and add accuracy report.
>
>
> +               *audio_ts = ns_to_timespec(nsec);
> +
> +               audio_tstamp_report->actual_type =
SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
> +               audio_tstamp_report->accuracy_report = 1; /* rest of
structure is valid */
> +               audio_tstamp_report->accuracy_m = 42; /* 24 MHz WallClock
== 42ns resolution */
> +               audio_tstamp_report->accuracy_e = 0;
> +

Do you mean wall clock run at 48000 * 500 Hz ? 42.666ns

Those aureal sound cards also can provide time stamp using VORTEX_SMP_TIME
mmio register which tick at 48000 * 256 Hz ?

Re: [PATCH v2 10/10] ALSA: hda: replace .wallclock by .get_time_info

[Pierre-Louis Bossart]

>> +               audio_tstamp_report->accuracy_m = 42; /* 24 MHz WallClock
> == 42ns resolution */
>> +               audio_tstamp_report->accuracy_e = 0;
>> +
>
> Do you mean wall clock run at 48000 * 500 Hz ? 42.666ns

HDAudio wall clock (and bit clock) runs at 24MHz. So yes that's 48000 * 500.

> Those aureal sound cards also can provide time stamp using VORTEX_SMP_TIME
> mmio register which tick at 48000 * 256 Hz ?

then it should be fairly easy to implement the same feature...

Re: [PATCH v2 00/10] audio timestamping evolutions

[Jaroslav Kysela]

Dne 19.12.2014 v 22:17 Pierre-Louis Bossart napsal(a):
> Thanks for the review Jaroslav
> 
>> 1) ext_info member is not required - the standard info field
>>     has enough free bits
> 
> Well this was added at Takashi's request, the initial patches didn't 
> rely on this extension...I can roll back those changes if this is the 
> consensus.

Yes, every developer has it's own opinions.. I would just not to add
next field until the all bits are not used.

>> 2) the whole struct snd_pcm_status is R/O -
>>     _IOR('A', 0x20, struct snd_pcm_status); I believe that it's much
>>     better to add new audio_tstamp_type to sw_params, but see (4)
> 
> I thought about this, but
> - selecting the timestamp type with sw_params would require multiple 
> system calls to achieve the same results. Every additional call or delay 
> changes the accuracy of the results and correlation between data 
> consumption and timing reports.
> - existing code already relies on snd_pcm_status to retrieve system and 
> audio timestamps, the selection through other means would make the code 
> more complicated.

Not much.. See bellow..

>> 3) accuracy - I would use uint64_t and report accuracy in pico-seconds
>>     (range from 0 picoseconds to 18446744 seconds); yes, use next bytes
>>     from the reserved part of status struct. the __u32 might be used only
>>     for flags
> 
> The timestamps are not better than nanoseconds. I don't actually know of 
> any link that uses a wallclock higher than 24/48Mhz, so that's already 
> ~20-40ns already. It seemed overkill to me do use more than 3 
> significant digits and an exponent to represent a nominal value that 
> doesn't take jitter and drift into account anyway. The idea was to 
> provide a qualitative value, not an actual measurement.

I just don't like the packing. I would use uint32 for nanoseconds or
eventually, it might be good to use numerator/denominator combo like for
the rate.

>> 4) if there is a motivation to call / obtain timestamps for multiple
>>     purposes (audio tstamp types), then offer to return all these
>>     timestamps in one shot rather than do multiple queries (again, use
>>     reserved bytes)
> 
> I thought about this case but I couldn't find any practical uses of 
> multiple timestamps at the same time. In the absence of any atomic 
> hardware snapshots of multiple counters, reading multiple values 
> sequentially from different counters would actually water-down the 
> accuracy and value of the timestamps returned. It's already hard-enough 
> to track a single pair of audio and system counters.

Then - why you argument for my (2) comment against sw_params/status
combo ? I also think that one type of audio timestamp is enough for
"almost all" application to compare the audio time with other time
sources. Eventually, we can add multiple audio timestamps to the status
structure and multiple audio timestamp type selectors to sw params and
do everything in one shot (status ioctl) - which is the best method
because you save time to retreve the other fields in the status
structure again. So - for example - I would agree to have 2 audio
timestamp selectors in sw_params and provide 2 different audio
timestamps in the status structure. This may be the ultimate solution.

					Jaroslav

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

Re: [PATCH v2 00/10] audio timestamping evolutions

[Takashi Iwai]

At Sun, 21 Dec 2014 14:14:42 +0100,
Jaroslav Kysela wrote:
> 
> Dne 19.12.2014 v 22:17 Pierre-Louis Bossart napsal(a):
> > Thanks for the review Jaroslav
> > 
> >> 1) ext_info member is not required - the standard info field
> >>     has enough free bits
> > 
> > Well this was added at Takashi's request, the initial patches didn't 
> > rely on this extension...I can roll back those changes if this is the 
> > consensus.
> 
> Yes, every developer has it's own opinions.. I would just not to add
> next field until the all bits are not used.

This was what we thought at first.  But later on, I found that it's a
bit ugly.  Namely, assume you'll add more tstamp types to be
advertised in INFO bit, but we already add yet another irrelevant bit
beforehand.  Then the whole type bits will become sparse.

Since this is a (possibly) extensible bitmask, having it separately
would make sense.

Though, I have no strong opinion on this.  If you have a good counter
argument, I'll be your ears :)


thanks,

Takashi

Re: [PATCH v2 00/10] audio timestamping evolutions

[Pierre-Louis Bossart]

On 12/21/14 7:14 AM, Jaroslav Kysela wrote:
> Dne 19.12.2014 v 22:17 Pierre-Louis Bossart napsal(a):
>> Thanks for the review Jaroslav
>>
>>> 1) ext_info member is not required - the standard info field
>>>      has enough free bits
>>
>> Well this was added at Takashi's request, the initial patches didn't
>> rely on this extension...I can roll back those changes if this is the
>> consensus.
>
> Yes, every developer has it's own opinions.. I would just not to add
> next field until the all bits are not used.

I kind of like Takashi's argument that all timestamp bits should remain 
together in a new field.

>
>>> 2) the whole struct snd_pcm_status is R/O -
>>>      _IOR('A', 0x20, struct snd_pcm_status); I believe that it's much
>>>      better to add new audio_tstamp_type to sw_params, but see (4)
>>
>> I thought about this, but
>> - selecting the timestamp type with sw_params would require multiple
>> system calls to achieve the same results. Every additional call or delay
>> changes the accuracy of the results and correlation between data
>> consumption and timing reports.
>> - existing code already relies on snd_pcm_status to retrieve system and
>> audio timestamps, the selection through other means would make the code
>> more complicated.
>
> Not much.. See bellow..
>
>>> 3) accuracy - I would use uint64_t and report accuracy in pico-seconds
>>>      (range from 0 picoseconds to 18446744 seconds); yes, use next bytes
>>>      from the reserved part of status struct. the __u32 might be used only
>>>      for flags
>>
>> The timestamps are not better than nanoseconds. I don't actually know of
>> any link that uses a wallclock higher than 24/48Mhz, so that's already
>> ~20-40ns already. It seemed overkill to me do use more than 3
>> significant digits and an exponent to represent a nominal value that
>> doesn't take jitter and drift into account anyway. The idea was to
>> provide a qualitative value, not an actual measurement.
>
> I just don't like the packing. I would use uint32 for nanoseconds or
> eventually, it might be good to use numerator/denominator combo like for
> the rate.

ok, i'll remove the packing. I was only trying to save space but if 
there are no issues reclaiming more bits then it's indeed more elegant.

>>> 4) if there is a motivation to call / obtain timestamps for multiple
>>>      purposes (audio tstamp types), then offer to return all these
>>>      timestamps in one shot rather than do multiple queries (again, use
>>>      reserved bytes)
>>
>> I thought about this case but I couldn't find any practical uses of
>> multiple timestamps at the same time. In the absence of any atomic
>> hardware snapshots of multiple counters, reading multiple values
>> sequentially from different counters would actually water-down the
>> accuracy and value of the timestamps returned. It's already hard-enough
>> to track a single pair of audio and system counters.
>
> Then - why you argument for my (2) comment against sw_params/status
> combo ? I also think that one type of audio timestamp is enough for
> "almost all" application to compare the audio time with other time
> sources. Eventually, we can add multiple audio timestamps to the status
> structure and multiple audio timestamp type selectors to sw params and
> do everything in one shot (status ioctl) - which is the best method
> because you save time to retreve the other fields in the status
> structure again. So - for example - I would agree to have 2 audio
> timestamp selectors in sw_params and provide 2 different audio
> timestamps in the status structure. This may be the ultimate solution.

I agree that the two-timestamp solution is fine on paper, but it'd still 
be too disruptive for platforms with DSPs and that's the reason why I 
suggested a dynamic in-band selection of the timestamp type.

What I have in mind for a typical time-aware application is as follows:

ThreadA is in charge of data handling. When it’s awaken (period-elapsed
or timer interrupt), it reads the status and provides/extracts the data 
needed. It would also handle sample-rate conversion or timer smoothing 
if needed.

ThreadB is unrelated to data transfers and queries an audio timestamp on 
a 'regular' application-defined interval, e.g. every second. The 
precision is typically higher in this case than in ThreadA and the 
timestamp used for A/V sync, network alignment, anything where time 
matters. This information might be used in the SRC/smoothing used by 
ThreadA as well.

Depending on hardware design, the timestamps may be generated by a 
simple register read or require IPC with the DSP. The latter case is a 
lot more costly and disruptive. By letting ThreadA select a timestamp 
that doesn't require any IPC, we'd minimize the amount of handshakes 
with the DSP and make an optimal use of the hardware. I wouldn't have 
started this work without a hardware-driven need, a one-size-fits-all 
approach doesn't work. If the extended precision results in 
delays/hand-shakes then we only want to use it when needed, not by default.

Hope this clarifies the proposal.
-Pierre