Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A2DPSink callbacks are not called properly #2751

Closed
SomebodyOdd opened this issue Jan 12, 2025 · 5 comments · Fixed by #2757
Closed

A2DPSink callbacks are not called properly #2751

SomebodyOdd opened this issue Jan 12, 2025 · 5 comments · Fixed by #2757
Labels
waiting for feedback Requires response from original poster

Comments

@SomebodyOdd
Copy link
Contributor

Hello! I'm trying to prototype a Bluetooth speaker with audio visualization, but I've stumbled on weird behavior of A2DPSink class.

Sketch code
#include <BluetoothAudio.h>

class DummyConsumer : public BluetoothAudioConsumer_ {
  virtual bool init(uint8_t channels, uint32_t samplerate, A2DPSink *a2dpSink) override {
    _channels = channels;
    _samplerate = samplerate;
    _a2dpSink = a2dpSink;
    _state = STATE_INITIALIZED;

    return true;
  }
  virtual void setVolume(uint8_t gain) override {
  }
  virtual void startStream() override {
    if (_state != STATE_INITIALIZED) {
      return;
    }
    _state = STATE_STREAMING;
  }
  virtual void stopStream() override {
    if (_state != STATE_STREAMING) {
      return;
    }
    _state = STATE_INITIALIZED;
  }

  virtual void close() override {
    if (_state == STATE_STREAMING) {
      stopStream();
    }
    _state = STATE_OFF;
  }
};


A2DPSink a2dp;
DummyConsumer consumer;

void volumeCB(void *param, int pct) {
  (void)param;
  Serial.printf("New volume: %d", pct);
  Serial.println();
}

void connectCB(void *param, bool connected) {
  (void)param;
  Serial.printf("Connected: %s", connected ? "true" : "false");
  Serial.println();
}

void playbackCB(void *param, A2DPSink::PlaybackStatus state) {
  (void)param;
  Serial.printf("Playback: %d", state);
  Serial.println();
}

void transmitCB(void *param) {
  (void)param;
  Serial.println("Transmit call");
}

void setup() {
  Serial.begin(115200);
  a2dp.setName("PicoW Boom 00:00:00:00:00:00");
  a2dp.setConsumer(&consumer);
  a2dp.onVolume(volumeCB);
  a2dp.onConnect(connectCB);
  a2dp.onPlaybackStatus(playbackCB);
  a2dp.onTransmit(transmitCB);
  a2dp.begin();
}

void loop() {
}

Connecting my Android phone to Pico W and playing around with some music and volume controls, I get following output:

Playback: 2
Playback: 1
Connected: true
Playback: 2
Playback: 1
Connected: true
Playback: 2
Playback: 1
Connected: true
Playback: 2
Playback: 1
Connected: true
Playback: 2

There are several oddities here:

  1. Connected callback is called multiple times, even though there is only one connection.
  2. Connected callback is called not when connection happened, but when sound starts. This is also why Playback is observed earlier, than a Connection callback.
  3. No volume callbacks ever happen when changing volume on a phone, regardless of it's "Media Volume Sync" setting
  4. No Transmit callbacks as well

Looking through A2DPSink.cpp code reveals that while onTransmit and onVolume callbacks does get saved in class fields, there are no calls to it at all! Is this intended? If yes, how could I get access to volume controls?

Also, since I need to do some processing on audio samples, there is no obvious way to either get audio data in a callback (since there is no relevant arguments in onTransmit callback) or by reading a stream of it (while A2DPSink is a Stream, all of it's Stream methods are effectively stubs).
Looking into BluetoothAudioConsumerI2S, it seems to me that it relies on callbacks from I2S to do its job, which means that I need to setup some external timer that will tick for samples every now and then so I could read samples from A2DPSink myself. I would prefer to either have a callback to that, or a read-only Stream so that I could process data as they arrive and not rely on I2S callbacks. Is there any way to do this that I miss here?

Thank you!

@earlephilhower
Copy link
Owner

Your running into a cut and paste error and the BTStack weirdness, I think.

1 and 2 is just what the BTStack library in the pico SDK is doing. We get a connect event, we call our callback. They're sending multiple events, sometimes not in what I'd consider logical order.

3 yeah, oops I forgot to callback from the AVRCP message handler. A PR would be welcome.

4 should be deleted. This is a sink not a source so it's not transmitting anything.

For your needs you need to implement a BluetoothAudioConsumer which will get the decompressed audio samples as they arrive. A real Stream was not implemented because that would imply yet one more later of buffering and memory usage. The audio output codecs already have this logic and memory management so it's pushed off to them.

@earlephilhower earlephilhower added the waiting for feedback Requires response from original poster label Jan 13, 2025
@SomebodyOdd
Copy link
Contributor Author

A PR would be welcome.

I'll see what I can do here, but no promises - coding for microcontrollers is still new to me =)

For your needs you need to implement a BluetoothAudioConsumer which will get the decompressed audio samples as they arrive.

Will it, tho? Examples of consumers that I can find (BluetoothAudioConsumerI2S.cpp, for example) suggests that consumer doesn't get any samples passively, rather they request them with _a2dpSink->playback_handler((int16_t *) buff, 32); (in case of BluetoothAudioConsumerI2S, by getting a callback from a I2S object).
I've tried just putting this into a loop(), but this gives me a lot of empty samples, from what I assume because of me calling it too quickly and running into buffer underflow. Does that mean I'll have to setup some kind of logic to not read too quickly, similar to what I2S does? Or am I missing some other way?

@earlephilhower
Copy link
Owner

Ah, I think you're correct there. It was a while ago when I wrote this, but it is driven off of the request side, not the source. That's what the BTStack folks and other stacks I've seen implement because BT is so unreliable and subject to frequent dropouts, and it allows for some frequency drift in playback vs. encode.

So you do need to pace this off of some event generator. But aren't you outputting the audio after doing something? If so, then in the I2S callback you register (i.e. nothing in loop, all interrupt driven) you'd fill from A2DP, process X samples, then dump them back with i2s.write(buff, size). You'd still implement the class above, but add some magic before i2s.write() in

void BluetoothAudioConsumerI2S::fill() {
int num_samples = _i2s->availableForWrite() / 2;
int16_t buff[32 * 2];
while (num_samples > 63) {
_a2dpSink->playback_handler((int16_t *) buff, 32);
num_samples -= 64;
for (int i = 0; i < 64; i++) {
buff[i] = (((int32_t)buff[i]) * _gain) >> 8;
}
_i2s->write((uint8_t *)buff, 64 * 2);
}
}

@SomebodyOdd
Copy link
Contributor Author

But aren't you outputting the audio after doing something?

Eventually - sure! For now - I'm tinkering without any sound output (DAC has not arrived yet), just (trying to) grab samples and calculate stuff, so callbacks would've been helpful to test.
There is also a case (which is arguably very niche) of outputting it into something that has no such callbacks - WiFi for example.
But I guess I could just mock it by using pins not connected to anything.

Anyway - I'll leave this issue open in case I (or someone else) manage to fix volume callbacks and PR.

@earlephilhower
Copy link
Owner

Your other option is to just make a new BTAudio class which doesn't do the 0-stuffing underflow bits.

while (request_frames) {
*(request_buffer++) = 0;
*(request_buffer++) = 0;
request_frames--;
}

You'd obviously need to return the read amount since it's not guaranteed anymore.

That's part of the reference class because you're theoretically pulling from the BT ring buffer at the same rate as it's being filled so you'd never really hit that except in case of drift. There are more advanced ways to correct (repeating samples if you detect you're undersupplying, or dropping if you're consistently too far ahead) but they're not implemented here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Requires response from original poster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants