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 :-)