Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH v4 00/10] audio timestamping evolutions
Date: Mon, 02 Feb 2015 12:03:34 +0100	[thread overview]
Message-ID: <s5hsieofud5.wl-tiwai@suse.de> (raw)
In-Reply-To: <1422662163-7391-1-git-send-email-pierre-louis.bossart@linux.intel.com>

At Fri, 30 Jan 2015 17:55:53 -0600,
Pierre-Louis Bossart wrote:
> 
> 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.

Thanks for the patches.  They look almost good to me.  There are a few
minor coding-style issues, but the implementation details appear good
enough.

I can merge at least the first four patches as an improvement of the
current code base.  The rest API change needs more reviews.  I'll take
a closer look at them later, but I think the new API design is almost
acceptable as is.


thanks,

Takashi


> 
> 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
> 
> V3/4:
> Addressed feedback from Jaroslav:
> no change to STATUS ioctl, new functionality introduced in STATUS_EXT
> ioctl with r/w params.
> bumped PCM protocol for detection on STATUS_EXT in userspace
> used 32-bit word for accuracy report, no mantissa/exponent packing
> rolled back info_ext changes, all INFO fields remain in same word
> 
> Merged comments from Liam (code simplifications)
> fixed packing
> 
> Added driver_tstamp field in case there is a delay in passing the 
> tstamp and audio_tstamp over IPC.
> 
> 
> 
> 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: 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
>   ALSA: bump PCM protocol to 2.0.13
> 
>  Documentation/sound/alsa/timestamping.txt | 200 ++++++++++++++++++++++++++++++
>  include/sound/pcm.h                       |  67 +++++++++-
>  include/uapi/sound/asound.h               |  36 +++++-
>  sound/core/pcm_compat.c                   |  36 +++++-
>  sound/core/pcm_lib.c                      |  88 ++++++++-----
>  sound/core/pcm_native.c                   |  58 ++++++++-
>  sound/pci/hda/hda_controller.c            |  43 +++++--
>  sound/usb/pcm.c                           |   9 ++
>  8 files changed, 478 insertions(+), 59 deletions(-)
>  create mode 100644 Documentation/sound/alsa/timestamping.txt
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

  parent reply	other threads:[~2015-02-02 11:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-30 23:55 [PATCH v4 00/10] audio timestamping evolutions Pierre-Louis Bossart
2015-01-30 23:55 ` [PATCH v4 01/10] ALSA: core: don't override timestamp unconditionally Pierre-Louis Bossart
2015-01-30 23:55 ` [PATCH v4 02/10] ALSA: core: allow for trigger_tstamp snapshot in .trigger Pierre-Louis Bossart
2015-02-02 10:48   ` Takashi Iwai
2015-01-30 23:55 ` [PATCH v4 03/10] ALSA: hda: read trigger_timestamp immediately after starting DMA Pierre-Louis Bossart
2015-02-02 10:49   ` Takashi Iwai
2015-01-30 23:55 ` [PATCH v4 04/10] ALSA: usb: update trigger timestamp on first non-zero URB submitted Pierre-Louis Bossart
2015-02-02 10:52   ` Takashi Iwai
2015-01-30 23:55 ` [PATCH v4 05/10] ALSA: core: selection of audio_tstamp type and accuracy reports Pierre-Louis Bossart
2015-01-31  7:37   ` Alexander E. Patrakov
2015-01-31 20:01     ` Pierre-Louis Bossart
2015-01-30 23:55 ` [PATCH v4 06/10] ALSA: core: pass audio tstamp config from userspace Pierre-Louis Bossart
2015-01-30 23:56 ` [PATCH v4 07/10] ALSA: core: pass audio tstamp config from userspace in compat mode Pierre-Louis Bossart
2015-01-30 23:56 ` [PATCH v4 08/10] ALSA: core: replace .wall_clock by .get_time_info Pierre-Louis Bossart
2015-01-30 23:56 ` [PATCH v4 09/10] ALSA: hda: replace .wallclock " Pierre-Louis Bossart
2015-01-30 23:56 ` [PATCH v4 10/10] ALSA: bump PCM protocol to 2.0.13 Pierre-Louis Bossart
2015-02-02 11:03 ` Takashi Iwai [this message]
2015-02-02 15:52   ` [PATCH v4 00/10] audio timestamping evolutions Pierre-Louis Bossart
2015-02-02 16:06     ` Takashi Iwai
2015-02-02 16:11       ` Pierre-Louis Bossart
2015-02-02 16:12         ` Takashi Iwai
2015-02-02 16:15       ` Jaroslav Kysela

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hsieofud5.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox