At Mon, 02 Feb 2015 09:52:35 -0600, Pierre-Louis Bossart wrote:
Thanks for the review Takashi.
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 will resubmit a new patchset for the coding style and true/false fixes. ETA end of week.
Good to hear. But what about trigger_tstamp_pending_update? I prefer avoiding the change in a common header structure unless really needed.
OTOH, if any other drivers need the same flag and it's referred in the PCM core code *later*, it's fine to add it now. Just make it clear if this is the case.
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.
You had comments only on patches 2,3 and 4, so I understand that you'd merge these four patches after the fixes?
Yes.
It's fine with me to spend more time on the API changes and deal with the patches in separate steps, the first four are really different in nature and only internal fixes without impact on userspace.
Right, that's my intention.
At least, I'd like to have the API changes reviewed by Jaroslav, who already raised some arguments.
thanks,
Takashi
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
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel