A2DP and AudioPlayer do not mix well

[Nawor3565]

Problem Description

Follow-up from #2151

When mixing a CallbackBuffer'ed A2DP stream with an AudioPlayer instance, there appear to be some quirks with how AudioPlayer is coded that cause things to not function properly.

For context, my testing script is designed to play a short drum sound over I2S when a button is pressed. It also acts as an A2DP sink, and if A2DP is active, it will mix the drum sound with the A2DP stream before sending the mixed audio to I2S. The mixing actually works perfectly while the sample is playing, but for a moment after the sample is finished, the A2DP stream stutters before returning to normal. I traced the issue back to the AudioPlayer and I was able to figure out a band-aid "fix" by making some changes to the copy() function:

size_t copy(size_t bytes, bool a2dp_stream_fix = false) {
    size_t result = 0;
    if (active) {
      ...
      // return silence when there was no data
      if (result < bytes && silence_on_inactive) {
        //LOGW("Ran out of file bytes, only got %u, so padding with %u bytes of silence", result, (bytes - result));
        writeSilence(bytes - result);
        if (a2dp_stream_fix) setActive(false); // Set inactive immediately if mixing with A2DP
      }

    } else {
      // e.g. A2DP should still receive data to keep the connection open
      if (silence_on_inactive) {
        //LOGW("Player not active, writing %u bytes of silence", bytes);
        writeSilence(bytes);
      }
    }
    return result;
  }

Normally, the function is set to return silent bytes whenever copy() is called but there are either not the requested number of bytes left in the audio file, or if the AudioPlayer is inactive. When the player is inactive, the function normally returns 1024 bytes of silence regardless of how many were requested to be copied, so I just changed it from writeSilence(1024) to writeSilence(bytes). This makes the mixing work correctly while the AudioPlayer is inactive, since it now returns the number of bytes needed to match up with the A2DP frame instead of a static 1024 bytes.

This didn't address the stuttering after the audio finished playing. I was able to figure out that, for reasons unknown to me, the AudioPlayer remains Active for a bit ever after the player reached the end of the audio file. Now, the function does just return silence in this case too, and it seems to even pad the correct amount of silence if, for example, 2000 bytes are requested but there are only 500 bytes of audio left. However, even though the correct amount of silent bytes appear to be returned here, it still causes the mixer output to stutter until the player goes Inactive. So, I just brute-forced this by setting it to inactive as soon as there are fewer bytes left in the file than requested to copy, which works, except that it breaks playback when not mixing the AudioPlayer with A2DP. So, I added an argument to the function so that I can selectively make the player immediately inactive only when A2DP is streaming. This works, but is definitely a hack and not the right way to go about this.

In addition, setSilenceOnInactive must be disabled. It looks like this is another case where a static number of silent bytes are copied (In this case, 2048 bytes thanks to the setActive() function). If enabled, it will interfere with the mixing for the same reason as the above, but without it I get the popping sound after the player stops.

Hopefully this is enough info to go off of. I'm also including the .wav file I'm using, in case there's something special about it that results in the issues I'm having. Thanks!!

sample.wav

Device Description

ESP32-Wroom dev board, PCM5102A DAC

Sketch

#include <BluetoothA2DPSink.h>
#include <AudioTools.h>
#include <AudioTools/AudioCodecs/CodecWAV.h>        // WAV decoder
#include <AudioTools/Disk/AudioSourceLittleFS.h>

// — Config —
constexpr char *startFilePath="/";
constexpr char* ext=".wav";
constexpr int TRIGGER_PIN = 21;
bool lastTriggerState = HIGH;
esp_a2d_audio_state_t bluetoothAudioState = ESP_A2D_AUDIO_STATE_SUSPEND;

BluetoothA2DPSink a2dp_sink;
I2SStream i2s;
OutputMixer<int16_t> mixer(i2s, 2);

CallbackStream cb;
BufferedStream buffered(cb);

AudioSourceLittleFS source(startFilePath, ext);
WAVDecoder wavDecoder;
AudioPlayer player(source, mixer, wavDecoder);

AudioInfo fmt{44100, 2, 16};


// A2DP callback: send A2DP frames into callback buffer
void onA2dpData(const uint8_t* data, uint32_t len) {
  buffered.write(data, len);
}


// Callback to change AudioPlayer output depending on whether A2DP is active or not.
// If A2DP is active, output to Mixer, otherwise output directly to I2S
void audio_state_changed(esp_a2d_audio_state_t state, void *ptr){
  Serial.println(a2dp_sink.to_str(state));
  bluetoothAudioState = state;

  if (state == ESP_A2D_AUDIO_STATE_STARTED) {
    player.setOutput(mixer);
  } else {
    player.setOutput(i2s);
  }
}


// with each A2DP write: write X bytes of A2DP data to Mixer index 0, and copy X
// bytes of AudioPlayer data to Mixer index 1. Then flush Mixer buffer to I2S
size_t onWrite(const uint8_t *data, size_t len){
  //LOGW("=>  onWrite: %u", len);
  mixer.write(0, data, len);
  mixer.setIndex(1);
  player.copy(len, true); // Call with custom A2DP fix
  mixer.flushMixer();
  return len;
}


void setup() {
  Serial.begin(115200);
  //AudioToolsLogger.begin(Serial, AudioToolsLogLevel::Info);

  pinMode(TRIGGER_PIN, INPUT_PULLUP);

  // A2DP sink config
  a2dp_sink.set_stream_reader(onA2dpData, false);
  a2dp_sink.set_auto_reconnect(true);
  a2dp_sink.set_on_audio_state_changed(audio_state_changed);
  a2dp_sink.start("a2dp-i2s");

  auto cfg = i2s.defaultConfig(TX_MODE);
  cfg.sample_rate = 44100;
  cfg.bits_per_sample = fmt.bits_per_sample;
  cfg.channels = fmt.channels;
  cfg.pin_bck = 18;
  cfg.pin_ws = 22;
  cfg.pin_data = 19;
  i2s.begin(cfg);

  mixer.setAutoIndex(false);
  mixer.begin();

  player.setSilenceOnInactive(true);
  player.setAutoFade(false);
  player.begin(0, false);
  player.setAutoNext(false);

  cb.setWriteCallback(onWrite);
}

void loop() {
 // Only copy in main loop when A2DP is NOT streaming
 if (bluetoothAudioState != ESP_A2D_AUDIO_STATE_STARTED) player.copy();

  // --- GPIO trigger check ---
  bool triggerState = digitalRead(TRIGGER_PIN);
  if (lastTriggerState == HIGH && triggerState == LOW) {
    Serial.println("Trigger: Playing sample once");

    player.setIndex(0); // Set to replay the same sample
    player.play();
  }
  lastTriggerState = triggerState;

  delay(5);
}

Other Steps to Reproduce

No response

What is your development environment (incl. core version info)

Arduino IDE 2.3.4

I have checked existing issues, discussions and online documentation

• I confirm I have checked existing issues, discussions and online documentation

[pschatzmann]

Stupid question: why would you use the AudioPlayer at all and not e.g. a much simpler FileLoop or even a MemoryStream ?
If you ignore the wav header you can read from from the wav file directly.

If you really want to play multiple files in sequence, did you try to set the timout for moving to the next file to 0

ps. Don't assume that you need to write the same amunt on index 1 like on index 0: but query the system for the filled buffer size of index 0 and index 1 and just provide the difference, to make sure that if there was a difference, things are equalling out...

[pschatzmann]

Here is a version that just copies the PCM data from the WAV file. I tested with an AudioKit with the SD drive using SDMMC:

#include "AudioTools.h"
#include "AudioTools/AudioLibs/AudioBoardStream.h"
#include "BluetoothA2DPSink.h"
#include "SD_MMC.h"

CallbackStream cb;
BufferedStream buffered(cb);
BluetoothA2DPSink a2dp_sink(buffered);
//I2SStream i2s;
AudioBoardStream i2s(AudioKitEs8388V1); // AudioKit i2s
OutputMixer<int16_t> mixer(i2s, 2);
File file; 
StreamCopy copier(i2s, file); // copy file to decoder
AudioActions action; // handle buttons/pins 

// handle button press:just reopen the file to start the output from pos 44
void actionKeyOn(bool active, int pin, void* ptr){
  if (active){
    Serial.println("key pressed");
    // prevent memory leak
    file.close();
    // wav start with a wav header
    File new_file = SD_MMC.open("/test/sample.wav");
    if (!new_file){
      Serial.println("Could not open file");
      return;
    }
    // ignore wav header
    new_file.seek(44);
    // make file available for playing
    file = new_file;
  }
}

// Callback to change output
void audio_state_changed(esp_a2d_audio_state_t state, void *ptr){
  Serial.println(a2dp_sink.to_str(state));
  if (state == ESP_A2D_AUDIO_STATE_STARTED) {
    // output via mixer
    copier.setActive(false);
  } else {
    // direct output via copier
    copier.setActive(true);
  }
}

// with each A2DP write: write X bytes of A2DP data to Mixer index 0, and copy open
// bytes from file to Mixer index 1. Then flush Mixer buffer to I2S
size_t onWrite(const uint8_t *data, size_t len){
  //LOGW("=>  onWrite: %u", len);
  // write data from A2DP
  mixer.write(0, data, len);

  // mix with file if data available, otherwise fill up with 0
  int bytes_open = mixer.available(0) - mixer.available(1);
  uint8_t data_from_file[bytes_open];
  memset(data_from_file, 0, bytes_open); // provide silence by default
  file.readBytes((char*)data_from_file, bytes_open); // read bytes if available
  mixer.write(1, data_from_file, bytes_open); // write data from file or silence

  mixer.flushMixer();
  return len;
}

void setup() {
  Serial.begin(115200);
  AudioToolsLogger.begin(Serial, AudioToolsLogLevel::Warning);

  // A2DP sink config
  a2dp_sink.set_on_audio_state_changed(audio_state_changed);
  a2dp_sink.start("a2dp-i2s");

  // setup i2s
  auto cfg = i2s.defaultConfig(TX_MODE);
  cfg.sd_active = false; // AudioKit: don't use SPI
  i2s.begin(cfg);

  // setup SD
  if (!SD_MMC.begin("/sdcard",true)){
    Serial.println("SD failed");
    stop();
  }

  // setup mixer
  mixer.begin();

  // setup mixing callback
  cb.setWriteCallback(onWrite);

  // setup action on pin 5 to play file
  action.add(5, actionKeyOn, AudioActions::ActiveLow); 

}

void loop() {
  action.processActions();
  if (!copier.copy())
    delay(10);
}

ps. I think your wav file has an issue at the end: even if I play it in Audacity, I can hear a pop at the end...
This works better:
sample1.wav

[Nawor3565]

Thank you!! I was wracking my brain trying to find a good way to write silence in the absence of more file bytes, but that way is pretty obvious...

I had originally used AudioPlayer since it already has features like volume control and automatic track switching which I plan to use a bit farther down the line, and I was thinking "why reinvent the wheel?". But now it seems like it makes more sense to just implement those things manually.

For the wav file, did you just add 10ms of silence or did you do a fade-out? My understanding is that the popping is due to an instant change between a non-zero magnitude and zero, so a "gradual" fade to zero would eliminate the pop. Does that sound accurate?

Just one more question, I want to add two more buttons that will play different samples, which can all play simultaneously. But, this would mean that the mixer must be used even when there's no A2DP stream to trigger the callback. To address this, would the simplest way be to just manually write zeros to the BufferedStream in place of the A2DP data? for example (just pseudo code):

void loop() {
  action.processActions();

  if (!A2DP_active) {
    uint8_t silent_placeholder[512];
    memset(silent_placeholder, 0, 512);
    buffered.write(silent_placeholder, 512);
  }
}

Thanks again for your help!!