On 8/12/20 9:55 AM, Takashi Iwai wrote:
On Wed, 12 Aug 2020 16:46:40 +0200,
Pierre-Louis Bossart wrote:
After doing some experiments, I think I can identify the problem more precisely.
1. aplay can not reproduce this issue because it writes samples
immediately when there are some space in the buffer. However, you can
add --test-position to see how the delay grows with period size 256.
aplay -Dhw:1,0 --period-size=256 --buffer-size=480 /dev/zero -d 1 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000
Hz, Stereo
Suspicious buffer position (1 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (2 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (3 total): avail = 0, delay = 2096, buffer = 512
...
Isn't this about the alignment of the buffer size against the period
size, not the period size itself? i.e. in the example above, the
buffer size isn't a multiple of period size, and DSP can't handle if
the position overlaps the buffer size in a half way.
If that's the problem (and it's an oft-seen restriction), the right
constraint is
snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
Takashi
Oh sorry for my typo. The issue happens no matter what buffer size is
set. Actually, even if I want to set 480, it will change to 512
automatically.
Suspicious buffer position (1 total): avail = 0, delay = 2064, buffer
= 512 <-this one is the buffer size
OK, then it means that the buffer size alignment is already in place.
And this large delay won't happen if you use period size 240?
Takashi
Yes! If I set the period size to 240, it will not print "Suspicious
buffer position ..."
So it sounds like DSP handles the delay report incorrectly.
Then it comes to another question: the driver supports both SOF and
SST. Is there the behavior difference between both DSPs wrt this
delay issue?
I still don't get what the issue is. The two following cases work fine
with the SST/Atom driver:
root@chrx:~# aplay -Dhw:0,0 --period-size=240 --buffer-size=480
/dev/zero -d 2 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000
Hz, Stereo
root@chrx:~# aplay -Dhw:0,0 --period-size=960 --buffer-size=4800
/dev/zero -d 2 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000
Hz, Stereo
What if with --period-size=256 --buffer-size=512 and --test-position?
Can you reproduce the problem in your side?
Yes indeed with the existing driver:
root@chrx:~# aplay -Dhw:0,0 --period-size=256 --buffer-size=512
/dev/zero -d 2 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000
Hz, Stereo
underrun!!! (at least 0.312 ms long)
underrun!!! (at least 0.326 ms long)
Suspicious buffer position (1 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (2 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (3 total): avail = 0, delay = 2080, buffer = 512
Suspicious buffer position (4 total): avail = 0, delay = 2080, buffer = 512
Suspicious buffer position (5 total): avail = 0, delay = 2096, buffer = 512
Suspicious buffer position (6 total): avail = 0, delay = 2096, buffer = 512
but the new constraint to force a 1ms step added in the patch1 should
preclude this from happening.
The existing code has this:
/* Make sure, that the period size is always even */
snd_pcm_hw_constraint_step(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_PERIODS, 2);
return snd_pcm_hw_constraint_integer(runtime,
SNDRV_PCM_HW_PARAM_PERIODS);
and with the addition of period size being a multiple of 1ms all
requirements should be met?
I also wonder what's really missing, too :)
BTW, I took a look back at the thread, and CRAS seems using a very
large buffer, namely:
[ 52.434791] sound pcmC1D0p: PERIOD_SIZE [240:240]
[ 52.434802] sound pcmC1D0p: BUFFER_SIZE [204480:204480]
yes, that's 852 periods and 4.260 seconds. Never seen such values :-)
