On Thu, Nov 16, 2017 at 19:49:36 +0100, Takashi Iwai wrote:
On Thu, 16 Nov 2017 19:11:37 +0100, Pierre-Louis Bossart wrote:
On 11/16/17 8:48 AM, Takashi Iwai wrote:
On Thu, 16 Nov 2017 13:43:57 +0100, Henrik Eriksson wrote:
This reverts a change that was part of a larger commit. That change broke timestamps in some application code for us as outlined in the log, and reverting it obviously fixes this. Since the original change did not explain why the behaviour was modified I can't speak for the effects on that use case.
So this is a regression and we need to address it.
I *guess* that the reason to update tstamp event at the same hwptr is that now we can get a more fine-grained tstamp. OTOH, we have to avoid the regression by that.
What about the (untested) patch like below? It skips the tstamp update as default but follows the new standard when a better tstamp mode is deployed.
update_audio_tstamp() only updates the audio timestamp when the type is DEFAULT already, but indeed update runtime->status->tstamp is updated unconditionally.
I have a bit of heartburn with the suggested solutions because the delay can change even with the same hw_ptr. Not updating the timestamps would be going back to the days where timing updates was dependent on DMA granularity.
Maybe we could make the tstamp update dependent on a change in audio_tstamp, e.g.
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index a93a4235a332..b606f3ea7a17 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -248,8 +248,10 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream, runtime->rate); *audio_tstamp = ns_to_timespec(audio_nsecs); }
runtime->status->audio_tstamp = *audio_tstamp;runtime->status->tstamp = *curr_tstamp;
if (runtime->status->audio_tstamp != *audio_tstamp) {
This should be
if (!timespec_equal(&runtime->status->audio_tstamp, audio_tstamp)) {
runtime->status->audio_tstamp = *audio_tstamp;runtime->status->tstamp = *curr_tstamp;}OK for me as long as it works. Henrik?
This appears to work for our use case.
How do I progress this, i.e., do I update my patch to use Pierre-Louis's fix? How should I attribute that?
Regards, /henrik