On Tue, 16 Jul 2019 05:57:51 +0200, Channaiah Vanitha (RBEI/ECF3) wrote:
Hello Takashi-san,
Can you please reply your feedback for below mail chain.
Best regards, Vanitha Channaiah RBEI/ECF3
From: Channaiah Vanitha (RBEI/ECF3) Sent: Tuesday, June 18, 2019 4:44 AM To: 'Takashi Iwai' tiwai@suse.de Cc: alsa-devel@alsa-project.org; Wischer Timo (ADITG/ESS) twischer@de.adit-jv.com Subject: RE: [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer size is less than 2 period size.
Hello Takashi-san,
Firstly, very sorry for the late reply.
The current condition was chosen because otherwise it'll cause underrun errors. If the round down is needed for avoiding errors, it should be changed, yes. Otherwise, it needs a careful evaluation.
If buffer=2*period, the chance to slip the update is quite high unless you align the start. And the instability with 2xperiod is the very reason we've added this hack at the beginning.
If this is the case, current condition would also solve under run errors for the condition buffer=2*period+1frame. For, buffer=2*period+1frame, snd_pcm_wait() is waiting for more than a period and this could lead to xruns. The chance to slip the condition is more in case of buffer=2*period-1frame. Because, after first write (one period write), avail is less than one period by 1 frame. It has to wait until next DMA interrupt. There is more chance of xruns. In case of buffer=2*period, after first write (one period write) avail is still one period size and there is less chance of xruns.
In anyway, the description in the patch doesn't match with the change. Please update it to fit with the actual change if we still need to take this change inevitably.
For buffer>=2*period, round down of slave pointers and for buffer=2*period-1frame, round up of slave pointers will avoid xruns. Which otherwise causes snd_pcm_wait() to block for more than a period time and leads to xruns.
Hmm, it's still not clear at all. Please repost the patch with a more elaborated and correct description that matches with the code change.
thanks,
Takashi