From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH RFC 5/9] ALSA: core: selection of audio_tstamp type and accuracy reports Date: Wed, 10 Dec 2014 23:27:09 +0100 Message-ID: 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 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id D472426060B for ; Wed, 10 Dec 2014 23:27:09 +0100 (CET) In-Reply-To: <5488BF96.708@linux.intel.com> 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: Pierre-Louis Bossart Cc: Nick Stoughton , alsa-devel@alsa-project.org, Tim Cussins List-Id: alsa-devel@alsa-project.org 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. > 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. thanks, Takashi