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