From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Cussins Subject: Re: [PATCH RFC 5/9] ALSA: core: selection of audio_tstamp type and accuracy reports Date: Tue, 16 Dec 2014 14:01:28 +0000 Message-ID: <54903B38.9050907@eml.cc> References: <1418077426-8309-1-git-send-email-pierre-louis.bossart@linux.intel.com> <1418077426-8309-6-git-send-email-pierre-louis.bossart@linux.intel.com> <5488827E.4030403@linux.intel.com> <5488BF96.708@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by alsa0.perex.cz (Postfix) with ESMTP id 7F31A26063B for ; Tue, 16 Dec 2014 15:01:32 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 2068920AF0 for ; Tue, 16 Dec 2014 09:01:30 -0500 (EST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai , Pierre-Louis Bossart Cc: Nick Stoughton , alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org Hi guys, Sorry for the (substantial) delay in responding to this :) On 10/12/14 22:27, Takashi Iwai wrote: > At Wed, 10 Dec 2014 15:48:06 -0600, > Pierre-Louis Bossart wrote: >> >>> - Another concern is the compatibility with the current wallclock >>> implementation. Judging from your patch, the audio_tstamp won't be >>> obtained from get_time_info callback in the default tstamp mode, >>> right? This may result in a regression, as currently the driver >>> always gives the h/w audio_tstamp when the driver supports the >>> wallclock. >> >> Is this that big of a deal? To the best of my knowledge this wallclk >> thing was implemented for HDaudio only when we were prototyping the new >> hardware, and I don't think we ended-up contributing the corresponding >> patches for PulseAudio. We've since realized that the wallclock can't be >> available in all cases and that we need this selection capability in a >> variety of cases. >> >> Also even if we kept the .wall_clock callback, the wallclock handling >> could be relative (start at zero) or absolute. I implemented a reset to >> zero on stream startup, since the counter is not maintained when the >> hardware is idle, but there are implementations where the wallclock is >> really absolute and not reset (see below). > > I'm not asking for keeping the wall_clock callback itself. The > requirement is the compatible kernel *behavior*. This is essentially > a MUST, especially when the backward compatibility isn't too > difficult to achieve. > > For example, leave the type zero = TSTAMP_TYPE_COMPAT or such, and > makes the PCM core and driver behaving as compatible as wall_clock. > This should be relatively easy. > > BTW, what if the driver doesn't support the requested tstamp type? > Isn't there any need to query the capability beforehand? > > >>> - Last but not least: we're receiving multiple enhancement requests >>> regarding tstamp at the very same time. This patchset conflicts >>> with Tim and Nick's start_at extention. >>> >>> I believe this can be resolved later, but let's discuss the ground >>> line at first: the requirement and influence on both changes. >> >> I am aware of this and it's why I posted my patches earlier than planned >> to avoid merging different concepts later, it's probably best to have >> compatibility from day1. > > Yes, absolutely. Agreed :) >> My proposal was to have a start_at functionality based on the timestamp >> definitions I suggested and keep audio and system timestamps separate >> rather than add mixed typestamps such as >> SND_PCM_TSTAMP_TYPE_AUDIO_WALLCLOCK. the code could be something like: >> >> start_at(typestamp type, timestamp subtype, timespec value ) { >> >> if (type == SYSTEM) { >> _start_using_hrtimers(subtype, value) // would be CLOCK_REALTIME, >> MONOTONIC, maybe RAW >> } else if (type == AUDIO) { >> if (subtype == ABSOLUTE_WALLCLOCK) // not reset on audio subsystem startup >> _start_using_hardware(value) >> else >> // not sure what to do with regular counters, probably bail. >> error; >> } >> >> That way you can set what sort of system timestamp and what sort of >> audio timestamp you want reported by snd_pcm_status, and you can >> independently select the start timestamp of your choice. > > OK, let's see how other guys receive this idea. This seems fine to me! > > thanks, > > Takashi >