[pulseaudio-discuss] [PATCH v11 02/11] bluetooth: Fix usage of RTP structures in SBC codec

[Pali Rohár]

On Sunday 16 June 2019 12:06:18 Tanu Kaskinen wrote:
> On Sun, 2019-06-02 at 17:25 +0200, Pali Rohár wrote:
> > Rename struct rtp_payload to rtp_sbc_payload as it is specific for SBC
> > codec. Add proper checks for endianity in rtp.h header and use uint8_t type
> > where appropriated. And because rtp_sbc_payload structure is not parsed by
> > decoder there is no support for fragmented SBC frames. Add warning for it.
> > ---
> >  src/modules/bluetooth/a2dp-codec-sbc.c | 16 ++++++----
> >  src/modules/bluetooth/rtp.h            | 58 +++++++++++++++++++---------------
> >  2 files changed, 42 insertions(+), 32 deletions(-)
> > 
> > diff --git a/src/modules/bluetooth/a2dp-codec-sbc.c b/src/modules/bluetooth/a2dp-codec-sbc.c
> > index f339b570d..6ab0b46cd 100644
> > --- a/src/modules/bluetooth/a2dp-codec-sbc.c
> > +++ b/src/modules/bluetooth/a2dp-codec-sbc.c
> > @@ -480,7 +480,7 @@ static void reset(void *codec_info) {
> >  
> >  static void get_buffer_size(void *codec_info, size_t link_mtu, size_t *decoded_buffer_size, size_t *encoded_buffer_size) {
> >      struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> > -    size_t rtp_size = sizeof(struct rtp_header) + sizeof(struct rtp_payload);
> > +    size_t rtp_size = sizeof(struct rtp_header) + sizeof(struct rtp_sbc_payload);
> >      size_t num_of_frames = (link_mtu - rtp_size) / sbc_info->frame_length;
> >  
> >      *decoded_buffer_size = num_of_frames * sbc_info->codesize;
> > @@ -510,14 +510,14 @@ static int reduce_encoder_bitrate(void *codec_info) {
> >  static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
> >      struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> >      struct rtp_header *header;
> > -    struct rtp_payload *payload;
> > +    struct rtp_sbc_payload *payload;
> >      uint8_t *d;
> >      const uint8_t *p;
> >      size_t to_write, to_encode;
> >      unsigned frame_count;
> >  
> >      header = (struct rtp_header*) output_buffer;
> > -    payload = (struct rtp_payload*) (output_buffer + sizeof(*header));
> > +    payload = (struct rtp_sbc_payload*) (output_buffer + sizeof(*header));
> >  
> >      frame_count = 0;
> >  
> > @@ -562,7 +562,7 @@ static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t
> >      } PA_ONCE_END;
> >  
> >      /* write it to the fifo */
> > -    memset(output_buffer, 0, sizeof(*header) + sizeof(*payload));
> > +    pa_memzero(output_buffer, sizeof(*header) + sizeof(*payload));
> >      header->v = 2;
> >  
> >      /* A2DP spec: "A payload type in the RTP dynamic range shall be chosen".
> > @@ -583,13 +583,17 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
> >      struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> >  
> >      struct rtp_header *header;
> > -    struct rtp_payload *payload;
> > +    struct rtp_sbc_payload *payload;
> >      const uint8_t *p;
> >      uint8_t *d;
> >      size_t to_write, to_decode;
> >  
> >      header = (struct rtp_header *) input_buffer;
> > -    payload = (struct rtp_payload*) (input_buffer + sizeof(*header));
> > +    payload = (struct rtp_sbc_payload*) (input_buffer + sizeof(*header));
> > +
> > +    /* TODO: Add support for decoding fragmented SBC frames */
> > +    if (payload->is_fragmented)
> > +        pa_log_warn("SBC frame is fragmented, decoding may fail");
> 
> If we don't currently support fragmented frames, I think it would be
> better to just flat out fail here.

Ok, that sounds better. Throw an error and stop decoding.

> I imagine we'll hit a fatal error
> soon anyway, but if by some miracle PulseAudio managed to decode the
> stream anyway (maybe the frames aren't fragmented after all), the
> syslog would potentially get flooded with these warnings.
> 

-- 
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/20190616/e018271f/attachment.sig>