* pcm_dshare calculates size incorrectly on buffer rollover
@ 2018-10-12 18:21 Brendan Shanks
2018-10-18 13:13 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Brendan Shanks @ 2018-10-12 18:21 UTC (permalink / raw)
To: alsa-devel
I'm working on an embedded system based on an Ambarella H1 SoC (32-bit ARM
Cortex A9). Audio playback through ALSA (and GStreamer) works fine when
playing to the raw hw device. When playing through dshare (my normal
configuration), playback stops after exactly 7 hours 16 minutes, for about
3 minutes. During the 3 minutes, the playback thread consumes an entire CPU
core. After the 3 minutes, GStreamer reports an xrun, recovers from it, and
playback goes back to normal.
After some debugging, the problem seems to be in
snd_pcm_dshare_sync_area(). When dshare->appl_ptr rolls over to 0, 'size'
becomes huge, 3036676576. 'slave_size' is also much bigger than it should
be, 1258291680. This is what 'size' is set to when the for loop starts, and
I believe the for loop then spends ~3 minutes copying a huge amount of
samples.
This also explains the 7h16m time, it's linked to the PCM boundary which is
1258291200. At 48 kHz, 1258291200 samples takes 7h16m54s.
I'm not sure what the fix should be though. Is this really a bug in dshare,
or are bad values being set somewhere else? GStreamer? Or maybe the
period/buffer size (480/9600) is causing problems?
Any advice is appreciated, my modified snd_pcm_dshare_sync_area() and the
output are below.
Brendan Shanks
$ more /proc/asound/card0/pcm1p/sub0/hw_params
access: MMAP_INTERLEAVED
format: S16_LE
subformat: STD
channels: 4
rate: 48000 (48000/1)
period_size: 480
buffer_size: 9600
$ more /proc/asound/card0/pcm1p/sub0/sw_params
tstamp_mode: ENABLE
period_step: 1
avail_min: 480
start_threshold: 1
stop_threshold: 1258291200
silence_threshold: 0
silence_size: 1258291200
boundary: 1258291200
--- output from running 'gst-launch-1.0 -e audiotestsrc ! alsasink' with
prints included as below
snd_pcm_dshare_sync_area size 3036676576, dshare->appl_ptr 0
dshare->last_appl_ptr 1258290720
slave_hw_ptr1 1258282080 slave_period_size 480
slave_hw_ptr2 1258282080 slave_buffer_size 9600
slave_hw_ptr3 1258291680 slave_boundary 1258291200
slave_hw_ptr4 1258291680 slave_appl_ptr 0
slave_size1 1258291680
size 1258291680
appl_ptr 9120 size 1258291680 boundary 1258291200
last_appl_ptr 0
slave_appl_ptr 0
dshare->slave_appl_ptr 480
exiting
snd_pcm_dshare_sync_area size 3036676576, dshare->appl_ptr 0
dshare->last_appl_ptr 1258290720
slave_hw_ptr1 8965920 slave_period_size 480
slave_hw_ptr2 8965920 slave_buffer_size 9600
slave_hw_ptr3 8975520 slave_boundary 1258291200
slave_hw_ptr4 8975520 slave_appl_ptr 8975040
slave_size1 480
size 480
appl_ptr 9120 size 480 boundary 1258291200
last_appl_ptr 0
slave_appl_ptr 8640
dshare->slave_appl_ptr 8975520
exiting
--- snd_pcm_dshare_sync_area() from alsa-lib-1.1.6 with prints added:
static void snd_pcm_dshare_sync_area(snd_pcm_t *pcm)
{
snd_pcm_direct_t *dshare = pcm->private_data;
snd_pcm_uframes_t slave_hw_ptr, slave_appl_ptr, slave_size;
snd_pcm_uframes_t appl_ptr, size;
const snd_pcm_channel_area_t *src_areas, *dst_areas;
int print = 0;
/* calculate the size to transfer */
size = dshare->appl_ptr - dshare->last_appl_ptr;
if (! size)
return;
if (size > 9600)
{
printf("snd_pcm_dshare_sync_area size %lu, dshare->appl_ptr
%lu dshare->last_appl_ptr %lu\n",
size, dshare->appl_ptr, dshare->last_appl_ptr);
print = 1;
}
slave_hw_ptr = dshare->slave_hw_ptr;
if (print) printf("slave_hw_ptr1 %lu slave_period_size %lu", slave_hw_ptr,
dshare->slave_period_size);
/* don't write on the last active period - this area may be cleared
* by the driver during write operation...
*/
slave_hw_ptr -= slave_hw_ptr % dshare->slave_period_size;
if (print) printf("slave_hw_ptr2 %lu slave_buffer_size %lu", slave_hw_ptr,
dshare->slave_buffer_size);
slave_hw_ptr += dshare->slave_buffer_size;
if (print) printf("slave_hw_ptr3 %lu slave_boundary %lu", slave_hw_ptr,
dshare->slave_boundary);
if (dshare->slave_hw_ptr > dshare->slave_boundary)
slave_hw_ptr -= dshare->slave_boundary;
if (print) printf("slave_hw_ptr4 %lu slave_appl_ptr %lu", slave_hw_ptr,
dshare->slave_appl_ptr);
if (slave_hw_ptr < dshare->slave_appl_ptr)
slave_size = slave_hw_ptr + (dshare->slave_boundary -
dshare->slave_appl_ptr);
else
slave_size = slave_hw_ptr - dshare->slave_appl_ptr;
if (print) printf("slave_size1 %lu", slave_size);
if (slave_size < size)
size = slave_size;
if (! size)
return;
if (print) printf("size %lu", size);
/* add sample areas here */
src_areas = snd_pcm_mmap_areas(pcm);
dst_areas = snd_pcm_mmap_areas(dshare->spcm);
appl_ptr = dshare->last_appl_ptr % pcm->buffer_size;
if (print) printf("appl_ptr %lu size %lu boundary %lu", appl_ptr, size,
pcm->boundary);
dshare->last_appl_ptr += size;
dshare->last_appl_ptr %= pcm->boundary;
if (print) printf("last_appl_ptr %lu", dshare->last_appl_ptr);
slave_appl_ptr = dshare->slave_appl_ptr % dshare->slave_buffer_size;
if (print) printf("slave_appl_ptr %lu", slave_appl_ptr);
dshare->slave_appl_ptr += size;
dshare->slave_appl_ptr %= dshare->slave_boundary;
if (print) printf("dshare->slave_appl_ptr %lu", dshare->slave_appl_ptr);
for (;;) {
snd_pcm_uframes_t transfer = size;
if (appl_ptr + transfer > pcm->buffer_size)
transfer = pcm->buffer_size - appl_ptr;
if (slave_appl_ptr + transfer > dshare->slave_buffer_size)
transfer = dshare->slave_buffer_size -
slave_appl_ptr;
share_areas(dshare, src_areas, dst_areas, appl_ptr,
slave_appl_ptr, transfer);
size -= transfer;
if (! size)
break;
slave_appl_ptr += transfer;
slave_appl_ptr %= dshare->slave_buffer_size;
appl_ptr += transfer;
appl_ptr %= pcm->buffer_size;
}
if (print) printf("exiting");
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: pcm_dshare calculates size incorrectly on buffer rollover
2018-10-12 18:21 pcm_dshare calculates size incorrectly on buffer rollover Brendan Shanks
@ 2018-10-18 13:13 ` Takashi Iwai
2019-02-11 19:49 ` Brendan Shanks
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2018-10-18 13:13 UTC (permalink / raw)
To: Brendan Shanks; +Cc: alsa-devel
On Fri, 12 Oct 2018 20:21:07 +0200,
Brendan Shanks wrote:
>
> I'm working on an embedded system based on an Ambarella H1 SoC (32-bit ARM
> Cortex A9). Audio playback through ALSA (and GStreamer) works fine when
> playing to the raw hw device. When playing through dshare (my normal
> configuration), playback stops after exactly 7 hours 16 minutes, for about
> 3 minutes. During the 3 minutes, the playback thread consumes an entire CPU
> core. After the 3 minutes, GStreamer reports an xrun, recovers from it, and
> playback goes back to normal.
>
> After some debugging, the problem seems to be in
> snd_pcm_dshare_sync_area(). When dshare->appl_ptr rolls over to 0, 'size'
> becomes huge, 3036676576. 'slave_size' is also much bigger than it should
> be, 1258291680. This is what 'size' is set to when the for loop starts, and
> I believe the for loop then spends ~3 minutes copying a huge amount of
> samples.
> This also explains the 7h16m time, it's linked to the PCM boundary which is
> 1258291200. At 48 kHz, 1258291200 samples takes 7h16m54s.
>
> I'm not sure what the fix should be though. Is this really a bug in dshare,
> or are bad values being set somewhere else? GStreamer? Or maybe the
> period/buffer size (480/9600) is causing problems?
Maybe some 32bit boundary overflow? I vaguely remember of it.
Takashi
> Any advice is appreciated, my modified snd_pcm_dshare_sync_area() and the
> output are below.
>
> Brendan Shanks
>
>
> $ more /proc/asound/card0/pcm1p/sub0/hw_params
> access: MMAP_INTERLEAVED
> format: S16_LE
> subformat: STD
> channels: 4
> rate: 48000 (48000/1)
> period_size: 480
> buffer_size: 9600
> $ more /proc/asound/card0/pcm1p/sub0/sw_params
> tstamp_mode: ENABLE
> period_step: 1
> avail_min: 480
> start_threshold: 1
> stop_threshold: 1258291200
> silence_threshold: 0
> silence_size: 1258291200
> boundary: 1258291200
>
> --- output from running 'gst-launch-1.0 -e audiotestsrc ! alsasink' with
> prints included as below
> snd_pcm_dshare_sync_area size 3036676576, dshare->appl_ptr 0
> dshare->last_appl_ptr 1258290720
> slave_hw_ptr1 1258282080 slave_period_size 480
> slave_hw_ptr2 1258282080 slave_buffer_size 9600
> slave_hw_ptr3 1258291680 slave_boundary 1258291200
> slave_hw_ptr4 1258291680 slave_appl_ptr 0
> slave_size1 1258291680
> size 1258291680
> appl_ptr 9120 size 1258291680 boundary 1258291200
> last_appl_ptr 0
> slave_appl_ptr 0
> dshare->slave_appl_ptr 480
> exiting
>
> snd_pcm_dshare_sync_area size 3036676576, dshare->appl_ptr 0
> dshare->last_appl_ptr 1258290720
> slave_hw_ptr1 8965920 slave_period_size 480
> slave_hw_ptr2 8965920 slave_buffer_size 9600
> slave_hw_ptr3 8975520 slave_boundary 1258291200
> slave_hw_ptr4 8975520 slave_appl_ptr 8975040
> slave_size1 480
> size 480
> appl_ptr 9120 size 480 boundary 1258291200
> last_appl_ptr 0
> slave_appl_ptr 8640
> dshare->slave_appl_ptr 8975520
> exiting
>
> --- snd_pcm_dshare_sync_area() from alsa-lib-1.1.6 with prints added:
> static void snd_pcm_dshare_sync_area(snd_pcm_t *pcm)
> {
> snd_pcm_direct_t *dshare = pcm->private_data;
> snd_pcm_uframes_t slave_hw_ptr, slave_appl_ptr, slave_size;
> snd_pcm_uframes_t appl_ptr, size;
> const snd_pcm_channel_area_t *src_areas, *dst_areas;
> int print = 0;
>
> /* calculate the size to transfer */
> size = dshare->appl_ptr - dshare->last_appl_ptr;
> if (! size)
> return;
> if (size > 9600)
> {
> printf("snd_pcm_dshare_sync_area size %lu, dshare->appl_ptr
> %lu dshare->last_appl_ptr %lu\n",
> size, dshare->appl_ptr, dshare->last_appl_ptr);
> print = 1;
> }
> slave_hw_ptr = dshare->slave_hw_ptr;
> if (print) printf("slave_hw_ptr1 %lu slave_period_size %lu", slave_hw_ptr,
> dshare->slave_period_size);
> /* don't write on the last active period - this area may be cleared
> * by the driver during write operation...
> */
> slave_hw_ptr -= slave_hw_ptr % dshare->slave_period_size;
> if (print) printf("slave_hw_ptr2 %lu slave_buffer_size %lu", slave_hw_ptr,
> dshare->slave_buffer_size);
> slave_hw_ptr += dshare->slave_buffer_size;
> if (print) printf("slave_hw_ptr3 %lu slave_boundary %lu", slave_hw_ptr,
> dshare->slave_boundary);
> if (dshare->slave_hw_ptr > dshare->slave_boundary)
> slave_hw_ptr -= dshare->slave_boundary;
> if (print) printf("slave_hw_ptr4 %lu slave_appl_ptr %lu", slave_hw_ptr,
> dshare->slave_appl_ptr);
> if (slave_hw_ptr < dshare->slave_appl_ptr)
> slave_size = slave_hw_ptr + (dshare->slave_boundary -
> dshare->slave_appl_ptr);
> else
> slave_size = slave_hw_ptr - dshare->slave_appl_ptr;
> if (print) printf("slave_size1 %lu", slave_size);
> if (slave_size < size)
> size = slave_size;
> if (! size)
> return;
> if (print) printf("size %lu", size);
>
> /* add sample areas here */
> src_areas = snd_pcm_mmap_areas(pcm);
> dst_areas = snd_pcm_mmap_areas(dshare->spcm);
> appl_ptr = dshare->last_appl_ptr % pcm->buffer_size;
> if (print) printf("appl_ptr %lu size %lu boundary %lu", appl_ptr, size,
> pcm->boundary);
> dshare->last_appl_ptr += size;
> dshare->last_appl_ptr %= pcm->boundary;
> if (print) printf("last_appl_ptr %lu", dshare->last_appl_ptr);
> slave_appl_ptr = dshare->slave_appl_ptr % dshare->slave_buffer_size;
> if (print) printf("slave_appl_ptr %lu", slave_appl_ptr);
> dshare->slave_appl_ptr += size;
> dshare->slave_appl_ptr %= dshare->slave_boundary;
> if (print) printf("dshare->slave_appl_ptr %lu", dshare->slave_appl_ptr);
> for (;;) {
> snd_pcm_uframes_t transfer = size;
> if (appl_ptr + transfer > pcm->buffer_size)
> transfer = pcm->buffer_size - appl_ptr;
> if (slave_appl_ptr + transfer > dshare->slave_buffer_size)
> transfer = dshare->slave_buffer_size -
> slave_appl_ptr;
> share_areas(dshare, src_areas, dst_areas, appl_ptr,
> slave_appl_ptr, transfer);
> size -= transfer;
> if (! size)
> break;
> slave_appl_ptr += transfer;
> slave_appl_ptr %= dshare->slave_buffer_size;
> appl_ptr += transfer;
> appl_ptr %= pcm->buffer_size;
> }
> if (print) printf("exiting");
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: pcm_dshare calculates size incorrectly on buffer rollover
2018-10-18 13:13 ` Takashi Iwai
@ 2019-02-11 19:49 ` Brendan Shanks
0 siblings, 0 replies; 3+ messages in thread
From: Brendan Shanks @ 2019-02-11 19:49 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
> On Oct 18, 2018, at 6:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
>
> On Fri, 12 Oct 2018 20:21:07 +0200,
> Brendan Shanks wrote:
>>
>> I'm working on an embedded system based on an Ambarella H1 SoC (32-bit ARM
>> Cortex A9). Audio playback through ALSA (and GStreamer) works fine when
>> playing to the raw hw device. When playing through dshare (my normal
>> configuration), playback stops after exactly 7 hours 16 minutes, for about
>> 3 minutes. During the 3 minutes, the playback thread consumes an entire CPU
>> core. After the 3 minutes, GStreamer reports an xrun, recovers from it, and
>> playback goes back to normal.
>>
>> After some debugging, the problem seems to be in
>> snd_pcm_dshare_sync_area(). When dshare->appl_ptr rolls over to 0, 'size'
>> becomes huge, 3036676576. 'slave_size' is also much bigger than it should
>> be, 1258291680. This is what 'size' is set to when the for loop starts, and
>> I believe the for loop then spends ~3 minutes copying a huge amount of
>> samples.
>> This also explains the 7h16m time, it's linked to the PCM boundary which is
>> 1258291200. At 48 kHz, 1258291200 samples takes 7h16m54s.
>>
>> I'm not sure what the fix should be though. Is this really a bug in dshare,
>> or are bad values being set somewhere else? GStreamer? Or maybe the
>> period/buffer size (480/9600) is causing problems?
>
> Maybe some 32bit boundary overflow? I vaguely remember of it.
I think I figured out the issue, it is in snd_pcm_dshare_sync_area() as I suspected. When ‘slave_hw_ptr’ rolled over the ‘slave_boundary’, the wrong ‘slave_hw_ptr’ variable was being compared, resulting in ‘slave_size’ and ‘size’ being much too large. It was likely only triggered on 32-bit systems, since the PCM boundary is computed based on LONG_MAX and is much larger on 64-bit systems.
It looks like this same fix was made to pcm_dmix in commit 6c7f60f7a982fdba828e4530a9d7aa0aa2b704ae ("Fix boundary overlap”) from June 2005. I’ll send a patch to the list shortly.
Brendan
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-11 19:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 18:21 pcm_dshare calculates size incorrectly on buffer rollover Brendan Shanks
2018-10-18 13:13 ` Takashi Iwai
2019-02-11 19:49 ` Brendan Shanks
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.