[pulseaudio-discuss] [PATCH v8 07/12] bluetooth: Add A2DP aptX and aptX HD codecs support

[Pali Rohár]

On Friday 19 April 2019 14:30:24 Tanu Kaskinen wrote:
> On Fri, 2019-04-19 at 12:52 +0200, Pali Rohár wrote:
> > On Friday 19 April 2019 11:41:22 Tanu Kaskinen wrote:
> > > On Sat, 2019-04-06 at 11:16 +0200, Pali Rohár wrote:
> > > > +static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
> > > > +    struct aptx_context *aptx_c = (struct aptx_context *) codec_info;
> > > > +
> > > > +    const uint8_t *p;
> > > > +    uint8_t *d;
> > > > +    size_t to_write, to_decode;
> > > > +
> > > > +    p = input_buffer;
> > > > +    to_decode = input_size;
> > > > +
> > > > +    d = output_buffer;
> > > > +    to_write = output_size;
> > > > +
> > > > +    while (PA_LIKELY(to_decode > 0)) {
> > > 
> > > Is it intentional that encode_buffer() checks both to_encode and
> > > to_write in the while loop condition, but decode_buffer() only checks
> > > to_decode? If so, why does encode_buffer() need to be extra careful but
> > > decode_buffer() doesn't?
> > 
> > These checks are similar/same as in SBC codec code. And SBC code was
> > there prior to my work. So personally I do not know.
> 
> Ok, let's find out if this code needs changing.
> 
> The output buffer size when decoding is determined by
> get_read_block_size(), which returns a value corresponding to the read
> MTU (the MTU is in the compressed domain, the block size is in the
> uncompressed domain).
> 
> The input buffer size is potentially up to twice the read MTU.
> 
> It doesn't seem impossible that the socket could have more data queued
> than just one MTU. Therefore this doesn't seem safe, because
> decode_buffer() may overrun the output buffer. The solution isn't to
> check to_write, however, because that would lead to some audio getting
> dropped. I think the input buffer size should be limited to roughly one
> MTU just like the output buffer size is limited to roughly one MTU. The
> exact logic is codec dependent, so the codec implementation should
> decide both the input and output buffer sizes, and it should be ensured
> that the output buffer is big enough to fully decode a full input
> buffer.

I'm looking at both pulseaudio encode/decode API and calling aptX
encode/decode can be simplified. I will update this code in next patch
version.

-- 
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/20190420/733e78ed/attachment.sig>