[pulseaudio-discuss] [PATCH v11 01/11] bluetooth: Fix A2DP codec API to provide information about data buffer

[Pali Rohár]

On Tuesday 18 June 2019 10:07:15 Tanu Kaskinen wrote:
> On Mon, 2019-06-17 at 12:05 +0200, Pali Rohár wrote:
> > On Saturday 15 June 2019 11:50:10 Tanu Kaskinen wrote:
> > > Although I made that mistake, I think I'm right in saying that our
> > > reading logic is broken at least with SBC. The sender can change the
> > > frame size without warning, so we shouldn't base our read (encoded)
> > > buffer size on that. If our buffer size is less than MTU (which it
> > > currently can be), the frame size may change in such a way that future
> > > packets are larger than our allocated read buffer. That will lead to
> > > reading partial packets.
> > > 
> > > This is what I think would be correct:
> > > 
> > > 1) Use the read MTU as the encoded data buffer size.
> > > 
> > > 2) After calling pa_read(), inspect the RTP header to find out the RTP
> > > packet payload size. If it's larger than what can fit our read buffer,
> > > that's an error, because the packets shouldn't exceed the MTU.
> > > 
> > > 3) Decode only the payload part, not the whole buffer.
> > > 
> > > 4) It's unfortunately possible (or so I think until proven otherwise)
> > > that there were two RTP packets queued in the socket, and the first one
> > > didn't fill the MTU completely, so we have the beginning of the second
> > > packet in our read buffer. If this is the case, we have to save the
> > > leftover part somewhere. That somewhere can be the beginning of the
> > > read buffer. The next time we read from the socket, we read using an
> > > offset so that the new data goes after the earlier leftover data.
> > > 
> > > 5) When the streaming stops, the leftover offset needs to be reset, so
> > > that it doesn't cause trouble when restarting streaming later.
> > 
> > I'm not sure if all this can happen. A2DP socket created by bluez, which
> > is passed to pulseaudio via dbus, is of type SOCK_SEQPACKET.
> > 
> > man 2 socket describes it as:
> > 
> >   SOCK_SEQPACKET  Provides a sequenced, reliable, two-way connection-
> >                   based data transmission path for datagrams of fixed
> >                   maximum length; a consumer is required to read an
> >                   entire packet with each input system call.
> > 
> >   SOCK_SEQPACKET sockets employ the same system calls as
> >   SOCK_STREAM sockets.  The only difference is that read(2) calls will
> >   return only the amount of data requested, and any data remaining in
> >   the arriving packet will be discarded.  Also all message boundaries
> >   in incoming datagrams are preserved.
> 
> Ok, that's great! There won't be any leftover data that we need to be
> concerned about. And if we call msgrcv() with 100 byte buffer and the
> frame is 70 bytes, we will get only 70 bytes. We just need to make sure
> that the read buffer is always big enough (equal to MTU should
> suffice).

Fixed in v12 that buffer has correct size.

-- 
Pali Rohár
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20190705/d16b0d67/attachment.sig>