[pulseaudio-discuss] [PATCH v0 1/3] bluetooth: Fix potential assertion failure due to unaligned packet size

[Tanu Kaskinen]

[Adding pulseaudio-discuss back to CC.]

On Thu, 2013-01-24 at 17:51 +0100, Mikel Astiz wrote:
> HI Tanu,
> 
> On Thu, Jan 24, 2013 at 2:46 PM, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > On Thu, 2013-01-24 at 10:16 +0100, Mikel Astiz wrote:
> >> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> >>
> >> While reading from the SCO socket, there is no guarantee regarding the
> >> resulting packet size. In some rare cases, it might not even match the
> >> alignment expected in pa_source_post(), resulting in an assertion
> >> failure inside pa_volume_memchunk():
> >>
> >> I: [alsa-sink] module-loopback.c: Could not peek into queue
> >> I: [alsa-sink] module-loopback.c: Could not peek into queue
> >> I: [alsa-sink] module-loopback.c: Could not peek into queue
> >> E: [bluetooth] sample-util.c: Assertion 'pa_frame_aligned(c->length, spec)' failed at pulsecore/sample-util.c:725, function pa_volume_memchunk(). Aborting.
> >>
> >> Program received signal SIGABRT, Aborted.
> >> [Switching to Thread 0x7fffda98f700 (LWP 8058)]
> >> 0x00007ffff6177935 in raise () from /lib64/libc.so.6
> >> Missing separate debuginfos, use: debuginfo-install alsa-lib-1.0.26-1.fc17.x86_64 dbus-libs-1.4.10-7.fc17.x86_64 flac-1.2.1-9.fc17.x86_64 glibc-2.15-58.fc17.x86_64 gsm-1.0.13-6.fc17.x86_64 json-c-0.10-2.fc17.x86_64 libICE-1.0.8-1.fc17.x86_64 libSM-1.2.1-1.fc17.x86_64 libX11-1.5.0-2.fc17.x86_64 libXau-1.0.6-3.fc17.x86_64 libXext-1.3.1-1.fc17.x86_64 libXi-1.6.1-1.fc17.x86_64 libXtst-1.2.0-3.fc17.x86_64 libogg-1.3.0-1.fc17.x86_64 libsndfile-1.0.25-2.fc17.x86_64 libtool-ltdl-2.4.2-3.1.fc17.x86_64 libudev-182-3.fc17.x86_64 libuuid-2.21.2-3.fc17.x86_64 libvorbis-1.3.3-1.fc17.x86_64 libxcb-1.9-1.fc17.x86_64 speex-1.2-0.14.rc1.fc17.x86_64
> >> (gdb) bt
> >>     at pulsecore/sample-util.c:723
> >> ---
> >>  src/modules/bluetooth/module-bluetooth-device.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
> >> index becf716..8d9c124 100644
> >> --- a/src/modules/bluetooth/module-bluetooth-device.c
> >> +++ b/src/modules/bluetooth/module-bluetooth-device.c
> >> @@ -662,6 +662,15 @@ static int hsp_process_push(struct userdata *u) {
> >>
> >>          pa_assert((size_t) l <= pa_memblock_get_length(memchunk.memblock));
> >>
> >> +        /* In some rare occasions, we might receive packets of a very strange
> >> +         * size. In these cases, in order to avoid an assertion failure due
> >> +         * to unaligned data, round the size down */
> >> +        if (!pa_frame_aligned(l, &u->sample_spec)) {
> >> +            pa_log_warn("SCO packet received of unaligned size: %zu", l);
> >> +            if ((l -= l % pa_frame_size(&u->sample_spec)) == 0)
> >> +                break;
> >> +        }
> >
> > This drops half a frame from the stream. Next time when we receive a
> > chunk of data, I'd guess that it will start with the second half of the
> > frame, but the code assumes that the received chunks start from a frame
> > boundary. That means audio corruption. Of course, it's possible that the
> > sender really sent only half a frame and will never send the rest of it,
> > there's no way for us to know...
> >
> > Corrupted audio is better than crashing, but would it be better to
> > disconnect from the device if it's sending bad data?
> 
> SCO data is not reliable and depending on the setup it could have no
> retransmission at all, so I believe there's not much we can do if a
> packet has been received partially, except for dropping the packet
> entirely.

Ok, so is it possible that the device is sending a correctly aligned
packet, but part of the packet is lost in transmission? In that case the
patch is good. The comment could mention that the likely reason for
unaligned data is that part of the packet was lost during transmission.

-- 
Tanu