On 12/10/14, 4:27 PM, 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.
if someone used alsa-lib with the .get_wall_clock(), the new user-space code will provide the same results as today, no change (wall clock if supported, hw_ptr otherwise). So the library compatibility is preserved.
I don't mind adding a compatible kernel behavior for HDAudio only, but is this really necessary?
BTW, what if the driver doesn't support the requested tstamp type? Isn't there any need to query the capability beforehand?
if the timestamp type requested is not supported then the logic defaults to using the hw_ptr, same behavior as today.
I added a set of INFO defines and the matching is_supported queries in alsa-lib. I just did a pretty dumb copy/paste/edit there, maybe we can refactor the code here with a single routine taking a type parameter. feedback welcome there.