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

[Tanu Kaskinen]

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

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk