pschatzmann / arduino-audio-tools

Arduino Audio Tools (a powerful Audio library not only for Arduino)
GNU General Public License v3.0
1.55k stars 237 forks source link

AudioPlayer to A2DPStream, player.stop() -> player.play(), can hear noise for about 1 sec #576

Closed BeijingUncle closed 1 year ago

BeijingUncle commented 1 year ago

if(player.isActive()) { player.stop(); } else { player.play(); }

...

void loop() { if (player.isActive()) { player.copy(); } else { delay(50); } }

pschatzmann commented 1 year ago

I think this is really an issue of the ESP32 I2S functionality and I am not sure if I want to address this in my generic code. It think it will help if, instead of just stopping to send audio data you provide some silence to I2S before you stop. I remember that this was working fine in some older Arduino ESP32 version.

BeijingUncle commented 1 year ago

How can I do that? Change the player's source(player.setStream()) to a SoundGenerator stream before player.stop()? Is there a example I can use?

pschatzmann commented 1 year ago

If you use the latest version you can just call outputstream.writeSilence(bytes) to add some silence. E.g. i2s.writeSilence(1024). I am not sure what minimum about would be required

BeijingUncle commented 1 year ago

If you use the latest version you can just call outputstream.writeSilence(bytes) to add some silence. E.g. i2s.writeSilence(1024). I am not sure what minimum about would be required

Master version does not work:

void togglePlay(Button2& btn) { if (player.isActive()) { out.writeSilence(1024 * 128); player.stop(); } else { player.play(); } }

BTW, the noise is with music playing AFTER player.play();

And master version also can make noise all the time while the release version does not with same mp3.

pschatzmann commented 1 year ago

Hmm, in this case the issue might be caused by your logic in the loop. Did you try to remove all your logic and just call player.play(); What is your output stream ?

BeijingUncle commented 1 year ago

Hmm, in this case the issue might be caused by your logic in the loop. Did you try to remove all your logic and just call player.play(); What is your output stream ?

My output stream is A2DP, here is the sketch:


include "AudioTools.h"

include "AudioLibs/AudioA2DP.h"

include "AudioLibs/AudioSourceSDFAT.h"

include "AudioCodecs/CodecMP3Helix.h"

define SD_CS 13

define SD_MISO 2

define SD_MOSI 15

define SD_CLK 14

struct Button { const uint8_t PIN; uint32_t numberKeyPresses; bool pressed; };

Button button1 = {37, 0, false};

float vol = 0.5;

SdSpiConfig sdcfg(SD_CS, DEDICATED_SPI, SD_SCK_MHZ(4)); AudioSourceSDFAT source("/", "mp3", sdcfg);

A2DPStream out; MP3DecoderHelix decoder; AudioPlayer player(source, out, decoder);

void ARDUINO_ISR_ATTR isr(void arg) { Button s = static_cast<Button*>(arg); s->numberKeyPresses += 1; s->pressed = true; }

void ARDUINO_ISR_ATTR isr() { button1.numberKeyPresses += 1; button1.pressed = true; }

void setup() { SPI.begin(SD_CLK, SD_MISO, SD_MOSI, SD_CS);

pinMode(button1.PIN, INPUT_PULLUP); attachInterruptArg(button1.PIN, isr, &button1, FALLING);

Serial.begin(115200);

AudioLogger::instance().begin(Serial, AudioLogger::Warning);

auto cfg = out.defaultConfig(TX_MODE); cfg.auto_reconnect = false; cfg.name = ""; out.begin(cfg);

player.setVolume(vol); player.begin(); }

void togglePlay() { if (player.isActive()) { player.stop(); } else { player.play(); } }

void loop() { player.copy();

if (button1.pressed) { togglePlay(); button1.pressed = false; } }

pschatzmann commented 1 year ago

The simplest way to deal with this if you just make sure that A2DP continues to get some empty data (in the loop) while the player does not provide any! If you don't do this the A2DP connection is stoping automatically...

BeijingUncle commented 1 year ago

The simplest way to deal with this if you just make sure that A2DP continues to get some empty data (in the loop) while the player does not provide any! If you don't do this the A2DP connection is stoping automatically...

And when I call player.play(), A2DP connection will reconnect automatically?

I did something like this? still ...

void loop() { btn1.loop(); btn2.loop(); btn3.loop();

if (player.isActive()) { player.copy(); } else { out.writeSilence(1024); } }

pschatzmann commented 1 year ago

Yes, I expect that should keep A2DP open when the player is stopped... If the break is getting too long when you resume you might consider to rate limit the output by adding some delays as well.

BeijingUncle commented 1 year ago

A2DP connection stop means disconnected with my BT speaker?

pschatzmann commented 1 year ago

Yes, this should keep the A2DP connected. If you stop sending data it automatically gets disconnected.

BeijingUncle commented 1 year ago

If disconnected automatically, should I receive a info like: [W] AudioA2DP.h : 300 - ==> state: Disconnected ?

BeijingUncle commented 1 year ago

I add some check:

void togglePlay(Button2& btn) { if (player.isActive()) { Serial.println("player.stop()"); player.stop(); } else { Serial.print("out.isConnected(): "); Serial.println(out.isConnected()); Serial.print("our.isReady(): "); Serial.println(out.isReady()); Serial.println("player.play()"); player.play(); } }

void loop() { btn1.loop(); btn2.loop(); btn3.loop();

player.copy(); }

and player.stop() then player.play(), I got the states:

16:40:09.104 -> [W] AudioA2DP.h : 300 - ==> state: Connecting 16:40:10.161 -> [W] AudioA2DP.h : 300 - ==> state: Connected 16:40:11.282 -> [W] AudioA2DP.h : 209 - is_a2dp_active -> true with 59904 bytes 16:40:48.755 -> player.stop() 16:43:27.861 -> out.isConnected(): 1 16:43:27.861 -> our.isReady(): 1 16:43:27.861 -> player.play()

Connection is active, but I still get the noise

pschatzmann commented 1 year ago

I tried a sketch with my recommendations and for me it is working perfectly:


#include "AudioTools.h"
#include "AudioLibs/AudioA2DP.h"
#include "AudioLibs/AudioSourceSDFAT.h"
#include "AudioCodecs/CodecMP3Helix.h"
#include "AudioLibs/AudioKit.h"

const char* startFilePath = "/";
const char* ext = "mp3";
AudioSourceSDFAT source(startFilePath, ext, PIN_AUDIO_KIT_SD_CARD_CS);
A2DPStream out;
MP3DecoderHelix decoder;
AudioPlayer player(source, out, decoder);
AudioActions actions;

static void actionStartStop(bool, int, void*) {
    player.setActive(!player.isActive());
}

void setup() {
  Serial.begin(115200);
  AudioLogger::instance().begin(Serial, AudioLogger::Warning);

  // start / stop with audiokit pin 1
  actions.add(PIN_KEY1, actionStartStop);

  // Setting up SPI if necessary with the right SD pins by calling
  SPI.begin(PIN_AUDIO_KIT_SD_CARD_CLK, PIN_AUDIO_KIT_SD_CARD_MISO, PIN_AUDIO_KIT_SD_CARD_MOSI, PIN_AUDIO_KIT_SD_CARD_CS);

  // setup output - We send the test signal via A2DP - so we conect to a Bluetooth Speaker
  auto cfg = out.defaultConfig(TX_MODE);
  cfg.name = "LEXON MINO L";  // set the device here. Otherwise the next available device is used for output
  //cfg.auto_reconnect = true;  // if this is use we just quickly connect to the last device ignoring cfg.name
  out.begin(cfg);

  // setup player
  player.setVolume(0.1);
  player.begin();
}

void loop() {
  if (!player.isActive()) {
    out.writeSilence(1024);
  }
  player.copy();
  actions.processActions();
}

I added the setActive() method so that less code is required, but your logic using start() and stop() is working as well. I also added the new method setSilenceOnInctive(bool active); so that the writeSilence() can be avoided in the sketch.

BeijingUncle commented 1 year ago

setSilenceOnInactive() is just great!

BeijingUncle commented 1 year ago

It drives me crazy :-(

I updated all libs to master, the noise continuous during music is playing.

I assume you have a ESP32-LyraT board, so I wrote a sdmmc version and tested on my ESP32-LyraT-mini and reproduce the problem.

I've uploaded my sketch and 2 mp3s I tested with, could you please have a look by any chance?

Thank you!

https://github.com/BeijingUncle/test-data/raw/main/test.zip

pschatzmann commented 1 year ago

I am getting confused. Is this still the original issue with the stop -> play ? If not I want to remind you that in another issue I told you that you should use the sketch using the Basic API which is more reliable.

BeijingUncle commented 1 year ago

I am getting confused. Is this still the original issue with the stop -> play ? If not I want to remind you that in another issue I told you that you should use the sketch using the Basic API which is more reliable.

I tried this example, also add decoder.setMaxFrameSize(2048), no luck with me.

BTW, What is your arduino-esp32 version? mine is 2.0.6.