[alsa-devel] pcm_dshare calculates size incorrectly on buffer rollover

[Takashi Iwai]

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 at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>