Re: [PATCH RFC 5/9] ALSA: core: selection of audio_tstamp type and accuracy reports

[Takashi Iwai <tiwai@suse.de>]

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