[pulseaudio-discuss] [PATCH v12 01/13] bluetooth: Fix usage of MTU, buffer sizes and return values of encode/decode methods

[Pali Rohár]

On Saturday 13 July 2019 10:14:22 Tanu Kaskinen wrote:
> On Fri, 2019-07-05 at 15:02 +0200, Pali Rohár wrote:
> >  /* Run from IO thread */
> >  static int a2dp_write_buffer(struct userdata *u, size_t nbytes) {
> >      int ret = 0;
> >  
> > +    if (PA_UNLIKELY(!nbytes)) {
> > +        u->write_index += (uint64_t) u->write_memchunk.length;
> > +        pa_memblock_unref(u->write_memchunk.memblock);
> > +        pa_memchunk_reset(&u->write_memchunk);
> > +        return 0;
> > +    }
> 
> Is this to handle a situation where the encoder accepts input but
> doesn't produce any output (the algorithmic delay thing you mentioned
> in the commit message)?

Yes.

> This can't happen with SBC, right?

Every non-realtime codec has algorithmic delay, including SBC. But
libsbc implementation handles this in way, that it returns silence in
first frames (instead of not return anything).

And e.g. libopenaptx returns zero buffer.

> I think there should be a comment explaining when nbytes can be zero.

Ok, I will add comment.

> > @@ -578,9 +592,8 @@ static int a2dp_process_push(struct userdata *u) {
> >          pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->decoder_sample_spec));
> >          pa_smoother_resume(u->read_smoother, tstamp, true);
> >  
> > -        pa_memblock_release(memchunk.memblock);
> > -
> > -        pa_source_post(u->source, &memchunk);
> > +        if (PA_LIKELY(memchunk.length))
> > +            pa_source_post(u->source, &memchunk);
> 
> I'm not sure about this. It sounds like it should be impossible that an
> encoded frame results in no decoded data, so maybe an assertion would
> make more sense?

This is now generic decode routine which calls "plugin" method for
decoding. And I expect that it can do anything. For sure that are
compression methods which in first bits transfer some table, tree or
other helper information. Just I do not know if there are widely used
audio codecs with such features.

And important it can happen in SBC. SBC in A2DP specification supports
fragmented frames and you can decode it after you receive all fragments.
So in worst case frame length is less then MTU and then you need to wait
for more packets until you are able to receive full SBC frame and
decode.

Just fragmented SBC frames are not supported yet.

I just wanted to have this encode and decode procedure to be generic for
any future codec.

Again I can add comment that codec is free to do anything with input
packets and produce also zero length decoded audio stream...

> On the other hand, a simple check like this can't do
> much harm, and if some codec indeed can return zero data, just skipping
> pa_source_post() in that case seems correct.

Yes.

-- 
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/20190713/3271a49e/attachment.sig>