Corrupted sound after extending sample size to 32bit

A-KL commented 1 week ago

Problem Description

Hello,

I'm trying to get this example Changing the Sample Size to 32 bits to work with my boards, but no luck.

I'm getting a crunchy, distorted sound from the Right channel and nothing from the Left. So far I tried two different DAC boards with several different pin configurations. Here is a small video that shows the problem: (warning loud sound!) https://youtube.com/shorts/Tbj9YHYC19o

Here is the full sketch (it's a platformio-based project): https://github.com/A-KL/esp32-projects/blob/main/sound/simpe-radio/src/main.cpp

Which esp32 board did you use for testing?

The serial log looks good:

[I] AudioStreamsConverter.h : 490 - begin 16 -> 32 bits
[I] AudioTypes.h : 128 - out: sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] AudioTypes.h : 128 -  sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] I2SConfigESP32.h : 80 - rx/tx mode: TX_MODE
[I] I2SConfigESP32.h : 81 - port_no: 0
[I] I2SConfigESP32.h : 82 - is_master: Master
[I] I2SConfigESP32.h : 83 - sample rate: 44100
[I] I2SConfigESP32.h : 84 - bits per sample: 16
[I] I2SConfigESP32.h : 85 - number of channels: 2
[I] I2SConfigESP32.h : 86 - signal_type: Digital
[I] I2SConfigESP32.h : 88 - i2s_format: I2S_STD_FORMAT
[I] I2SConfigESP32.h : 90 - auto_clear: true
[I] I2SConfigESP32.h : 92 - use_apll: true
[I] I2SConfigESP32.h : 97 - buffer_count:6
[I] I2SConfigESP32.h : 98 - buffer_size:512
[I] I2SConfigESP32.h : 103 - pin_bck: 13
[I] I2SConfigESP32.h : 105 - pin_ws: 15
[I] I2SConfigESP32.h : 107 - pin_data: 12
[I] URLStream.h : 82 - virtual bool audio_tools::URLStream::begin(const char*, const char*, MethodID, const char*, const char*): http://stream.srg-ssr.ch/m/rsj/mp3_128
[I] Url.h : 58 - Url::parse
[I] Url.h : 96 - url->http://stream.srg-ssr.ch/m/rsj/mp3_128
[I] Url.h : 97 - host->stream.srg-ssr.ch
[I] Url.h : 98 - protocol->http
[I] Url.h : 99 - path->/m/rsj/mp3_128
[I] Url.h : 100 - port->80
[I] URLStream.h : 400 - bool audio_tools::URLStream::login()
.
[I] URLStream.h : 374 - WiFiClient
[I] HttpRequest.h : 220 - process connecting to host stream.srg-ssr.ch port 80
[I] HttpRequest.h : 340 - is connected true  with timeout 60000
[I] HttpRequest.h : 231 - Free heap: 222244
[I] HttpHeader.h : 276 - HttpHeader::write
[I] HttpHeader.h : 421 - -> GET /m/rsj/mp3_128 HTTP/1.1
[I] HttpHeader.h : 210 -  -> Host: stream.srg-ssr.ch 
[I] HttpHeader.h : 210 -  -> Connection: keep-alive 
[I] HttpHeader.h : 210 -  -> Accept-Encoding: identity 
[I] HttpHeader.h : 210 -  -> Accept: audio/mp3 
[I] HttpHeader.h : 342 -  -> <CR LF> 
[I] HttpRequest.h : 291 - Request written ... waiting for reply
[I] HttpHeader.h : 244 - Waiting for data...
[I] HttpHeader.h : 253 - Data available: 5744
[I] HttpRequest.h : 164 - no CONTENT_LENGTH found in reply
[I] URLStream.h : 90 - size: 0
[I] URLStream.h : 97 - ==> http status: 200
[I] AudioEncoded.h : 75 - virtual void audio_tools::EncodedAudioOutput::addNotifyAudioChange(audio_tools::AudioInfoSupport&)
[I] StreamCopy.h : 158 - StreamCopy::copy  1024 -> 1024 -> 1024 bytes - in 1 hops
[I] AudioTypes.h : 128 - MP3DecoderHelix sample_rate: 48000 / channels: 2 / bits_per_sample: 16
[I] AudioStreamsConverter.h : 454 - -> NumberFormatConverterStream:
[I] AudioTypes.h : 128 - in: sample_rate: 48000 / channels: 2 / bits_per_sample: 16
[I] AudioTypes.h : 128 - out: sample_rate: 48000 / channels: 2 / bits_per_sample: 32
[I] StreamCopy.h : 158 - StreamCopy::copy  1024 -> 1024 -> 1024 bytes - in 1 hops
....

Device Description

LilyGO TTGO T-Display V1.1 ESP32 (with 1.14 inch TFT Display). I've other boards but it doesn't look the problem is board-dependent: https://github.com/Xinyuan-LilyGO/TTGO-T-Display
ES9018K2M DAC Audio Decoder Module: I2S: https://nl.aliexpress.com/item/1005005759804262.html (MCK not connected)
Es9018 K 2M Es9018 I 2S Dac Decoder Board: I2S: https://nl.aliexpress.com/item/1005005850295113.html

Sketch

#include "Creds.h"
#include "AudioTools.h"
#include "AudioTools/AudioCodecs/CodecMP3Helix.h"

URLStream url(LOCAL_SSID, LOCAL_PASSWORD);
I2SStream i2s; // final output of decoded stream
NumberFormatConverterStream nfc(i2s);
EncodedAudioStream dec(&nfc, new MP3DecoderHelix()); // Decoding stream
StreamCopy copier(dec, url); // copy url to decoder

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

  // convert 16 bits to 32, you could also change the gain
  nfc.begin(16, 32);

  // setup i2s
  auto config = i2s.defaultConfig(TX_MODE);
  // you could define e.g your pins and change other settings
  config.pin_ws = I2S_WS;
  config.pin_bck = I2S_BCK;
  config.pin_data = I2S_SD;
  //config.pin_mck = I2S_MCLK;
  //config.mode = I2S_STD_FORMAT;
  //config.bits_per_sample = 32; // we coult do this explicitly
  i2s.begin(config);

  // setup I2S based on sampling rate provided by decoder
  dec.begin();

// mp3 radio
  url.begin("http://stream.srg-ssr.ch/m/rsj/mp3_128","audio/mp3");

}

void loop(){
  copier.copy();
}

Other Steps to Reproduce

No response

What is your development environment

PlatformIO

I have checked existing issues, discussions and online documentation

[X] I confirm I have checked existing issues, discussions and online documentation

pschatzmann commented 1 week ago

You are crazy to report this as an issue: Please read the Wiki!

The most important basics
It's not working

This can't work with the log level Info!

A-KL commented 1 week ago

Thanks for the response. I wish it was that simple (maybe it still is though). Despite ending up using the original sketch as an example, as well as looking into documentation (like https://github.com/pschatzmann/arduino-audio-tools/wiki/Converting-the-Data-Format, https://github.com/pschatzmann/arduino-audio-tools/wiki/It's-not-working) - I also tried the following modifications:

AudioLogger::instance().begin(Serial, AudioLogger::Error);
Defining: NO_TRACE, LOG_NO_MSG, COPY_LOG_OFF, LOG_LEVEL AudioLogger::Error
CORE_DEBUG_LEVEL=1
USE_AUDIO_LOGGING=false, however, this one breaks the build, need to investigate more

However, there is no difference at all in the sound.

I would really appreciate it if you could point out what else needs to be modified in the original sketch or which parts of the documentation I've completely missed. Thanks!

pschatzmann commented 1 week ago

Just using AudioLogger::instance().begin(Serial, AudioLogger::Error); should be good enough Actually using the new conventions it should rather be AudioToolsLogger.begin(Serial, AudioToolsLogLevel::Error);

pschatzmann commented 1 week ago

Did you test with a sine generator test sketch that your board supports the 32 bits properly ?

pschatzmann commented 1 week ago

I confirm that for me it is not working any more as well...

A-KL commented 1 week ago

Thanks for the update!

I'm still running different tests. Switched to the board that doesn't have MCK input and can confirm that the sine generator sketch works.

Will now try 16 bit + NumberFormatConverterStream and see if that's where sound gets corrupted.

pschatzmann commented 1 week ago

One of the issues is that I2S does not get notified about the right values. You can try to add nfc.addNotifyAudioChange(i2s);

A-KL commented 1 week ago

A bit weird, with nfc.addNotifyAudioChange(i2s); I get no sound at all:

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

  nfc.addNotifyAudioChange(i2s);
  nfc.begin(16, 32);

  auto config = i2s.defaultConfig(TX_MODE);
  config.pin_ws = I2S_WS;
  config.pin_bck = I2S_BCK;
  config.pin_data = I2S_SD;
  i2s.begin(config);

  dec.begin();

  url.begin("http://stream.srg-ssr.ch/m/rsj/mp3_128","audio/mp3");

However, the sinewave example still gives corrupt sound:

AudioInfo info(44100, 2, 16);
SineWaveGenerator<int16_t> sineWave(32000);
GeneratedSoundStream<int16_t> sound(sineWave);
I2SStream out; 
NumberFormatConverterStream nfc(out);
StreamCopy copier(nfc, sound);

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

  nfc.addNotifyAudioChange(out);
  nfc.begin(16, 32);

  auto config = out.defaultConfig(TX_MODE);
  config.copyFrom(info); 

  config.pin_ws = I2S_WS;
  config.pin_bck = I2S_BCK;
  config.pin_data = I2S_SD;

  out.begin(config);

  sineWave.begin(info, N_B4);

There must be a mistake somewhere 😞 Did it fix the issue for you?

pschatzmann commented 1 week ago

No, but I suggest that you change the log level back to info, so that you can see wha't going on...

A-KL commented 1 week ago

I see that the output stays 16bit:

AudioStreamsConverter.h : 490 - begin 16 -> 32 bits
starting I2S...
[I] AudioTypes.h : 128 - out: sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] AudioTypes.h : 128 -  sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] I2SConfigESP32.h : 80 - rx/tx mode: TX_MODE
[I] I2SConfigESP32.h : 81 - port_no: 0
[I] I2SConfigESP32.h : 82 - is_master: Master
[I] I2SConfigESP32.h : 83 - sample rate: 44100
[I] I2SConfigESP32.h : 84 - bits per sample: 16
[I] I2SConfigESP32.h : 85 - number of channels: 2
[I] I2SConfigESP32.h : 86 - signal_type: Digital
[I] I2SConfigESP32.h : 88 - i2s_format: I2S_STD_FORMAT
[I] I2SConfigESP32.h : 90 - auto_clear: true
[I] I2SConfigESP32.h : 92 - use_apll: true
[I] I2SConfigESP32.h : 97 - buffer_count:6
[I] I2SConfigESP32.h : 98 - buffer_size:512
[I] I2SConfigESP32.h : 103 - pin_bck: 13
[I] I2SConfigESP32.h : 105 - pin_ws: 15
[I] I2SConfigESP32.h : 107 - pin_data: 12
[I] SoundGenerator.h : 165 - SineWaveGenerator::begin(channels=2, sample_rate=44100, frequency=493.88)
[I] SoundGenerator.h : 149 - bool audio_tools::SineWaveGenerator<T>::begin() [with T = short int]
[I] AudioTypes.h : 128 - SoundGenerator: sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] Buffers.h : 376 - resize: 4
[I] SoundGenerator.h : 192 - setFrequency: 493.88
[I] SoundGenerator.h : 193 - active: true
started...
[I] StreamCopy.h : 158 - StreamCopy::copy  1024 -> 1024 -> 1024 bytes - in 1 hops
...

However, if I explicitly set it to 32 via config.bits_per_sample = 32; the sound disappears.

[I] AudioStreamsConverter.h : 490 - begin 16 -> 32 bits
starting I2S...
[I] AudioTypes.h : 128 - in: sample_rate: 44100 / channels: 2 / bits_per_sample: 32
[I] AudioTypes.h : 128 - out: sample_rate: 44100 / channels: 2 / bits_per_sample: 32
[I] AudioTypes.h : 128 -  sample_rate: 44100 / channels: 2 / bits_per_sample: 32
[I] I2SConfigESP32.h : 80 - rx/tx mode: TX_MODE
[I] I2SConfigESP32.h : 81 - port_no: 0
[I] I2SConfigESP32.h : 82 - is_master: Master
[I] I2SConfigESP32.h : 83 - sample rate: 44100
[I] I2SConfigESP32.h : 84 - bits per sample: 32
[I] I2SConfigESP32.h : 85 - number of channels: 2
[I] I2SConfigESP32.h : 86 - signal_type: Digital
[I] I2SConfigESP32.h : 88 - i2s_format: I2S_STD_FORMAT
[I] I2SConfigESP32.h : 90 - auto_clear: true
[I] I2SConfigESP32.h : 92 - use_apll: true
[I] I2SConfigESP32.h : 97 - buffer_count:6
[I] I2SConfigESP32.h : 98 - buffer_size:512
[I] I2SConfigESP32.h : 103 - pin_bck: 13
[I] I2SConfigESP32.h : 105 - pin_ws: 15
[I] I2SConfigESP32.h : 107 - pin_data: 12
[I] SoundGenerator.h : 165 - SineWaveGenerator::begin(channels=2, sample_rate=44100, frequency=493.88)
[I] SoundGenerator.h : 149 - bool audio_tools::SineWaveGenerator<T>::begin() [with T = short int]
[I] AudioTypes.h : 128 - SoundGenerator: sample_rate: 44100 / channels: 2 / bits_per_sample: 16
[I] Buffers.h : 376 - resize: 4
[I] SoundGenerator.h : 192 - setFrequency: 493.88
[I] SoundGenerator.h : 193 - active: true
started...
[I] StreamCopy.h : 158 - StreamCopy::copy  1024 -> 1024 -> 1024 bytes - in 1 hops

Another thing I want to try is to run these examples of the latest main instead of v1.0.0 tag.

pschatzmann commented 1 week ago

I committed a correction that should fix the wrong conversion: it was clipping the data wrongly! For the missing automatic notification I will have another look tomorrow...

A-KL commented 1 week ago

Thanks a lot! The NumberFormatConverterStream now works for the radio sketch 🙌 No rush at all, just let me know and I'll test the rest.

pschatzmann commented 1 week ago

I also changed the logic to use the standard notification setup in this case as well, so this scenario will work as expected, with the correct sample rate (without calling addNotifiyAudioChange())

A-KL commented 5 days ago

Hi @pschatzmann, just want to confirm that both sketches work now with one of the Es DACs I have.

I'm a bit surprised that 'TeraDak' (the smaller purple board) still produces noise or no sound at all. Even with MCLK supplied, and every single combination of 16\24\32bit and 44.1\48\96\192kHz used.

It feels like the board might be defective. Especially, if yours works fine now. I've ordered another 32bit dac to run the same tests. Will also try attaching I2C and fiddling with settings.

Thanks a lot for the quick help! 🍺

pschatzmann commented 5 days ago

Maybe you need to set another I2S format

josef2600 commented 5 days ago

because it is almost the same code, i put it here. i use the sample bt_music_receiver_32bits , but the boards doesn't boot and gets restart. the code:

#define AUDIOKIT_BOARD  7

#include "AudioTools.h"
#include "AudioTools/AudioLibs/A2DPStream.h" // install https://github.com/pschatzmann/ESP32-A2DP
#include "AudioTools/AudioLibs/AudioBoardStream.h" // install https://github.com/pschatzmann/arduino-audio-driver
#include "BluetoothA2DPSink.h"

AudioBoardStream kit(AudioKitEs8388V2);  //AudioBoard.h  static AudioBoard AudioKitEs8388V2{AudioDriverES8388, PinsAudioKitEs8388v2};
NumberFormatConverterStream convert(kit);
BluetoothA2DPSink a2dp_sink(convert);
AudioInfo info(44100, 2, 24);

void setup()
{
  Serial.begin(115200);
  delay(500);
  AudioLogger::instance().begin(Serial, AudioLogger::Info);
  delay(100);
  // setup output
  auto cfg = kit.defaultConfig(TX_MODE);
  cfg.sd_active = false;
  cfg.copyFrom(info);
  kit.begin(cfg);
  delay(10);
  a2dp_sink.start("AudioKit");
  delay(100);
  convert.begin(16, 24);
  delay(100);
}

uint8_t vvv = 0;
int vvv2 = 0;
uint8_t vvv3 = 0;

void loop()
{
  delay(100);
}

and the log is:

Warning: Pin '23' not set up because of conflict
Warning: Pin '18' not set up because of conflict
[I] AudioTypes.h : 128 - in: sample_rate: 44100 / channels: 2 / bits_per_sample: 24
[I] AudioTypes.h : 128 - out: sample_rate: 44100 / channels: 2 / bits_per_sample: 24
[I] AudioTypes.h : 128 -  sample_rate: 44100 / channels: 2 / bits_per_sample: 24
[I] I2SConfigESP32.h : 80 - rx/tx mode: TX_MODE
[I] I2SConfigESP32.h : 81 - port_no: 0
[I] I2SConfigESP32.h : 82 - is_master: Master
[I] I2SConfigESP32.h : 83 - sample rate: 44100
[I] I2SConfigESP32.h : 84 - bits per sample: 24
[I] I2SConfigESP32.h : 85 - number of channels: 2
[I] I2SConfigESP32.h : 86 - signal_type: Digital
[I] I2SConfigESP32.h : 88 - i2s_format: I2S_STD_FORMAT
[I] I2SConfigESP32.h : 90 - auto_clear: true
[I] I2SConfigESP32.h : 92 - use_apll: true
[I] I2SConfigESP32.h : 97 - buffer_count:6
[I] I2SConfigESP32.h : 98 - buffer_size:512
[I] I2SConfigESP32.h : 101 - pin_mck: 0
[I] I2SConfigESP32.h : 103 - pin_bck: 5
[I] I2SConfigESP32.h : 105 - pin_ws: 25
[I] I2SConfigESP32.h : 107 - pin_data: 26

assert failed: bool audio_tools::NumberFormatConverterStream::begin(int, int, float) AudioStreamsConverter.h:487 (to_bit_per_samples > 0)

Backtrace: 0x40084101:0x3ffcceb0 0x40096acd:0x3ffcced0 0x4009cfd1:0x3ffccef0 0x400d8d52:0x3ffcd020 0x400d9197:0x3ffcd050 0x401cad5e:0x3ffcd070 0x401cb5a3:0x3ffcd090 0x400db3c1:0x3ffcd0b0 0x401cbad6:0x3ffcd0d0 0x400dbdbd:0x3ffcd0f0 0x400d915d:0x3ffcd130 0x400e326a:0x3ffcd210

it wasn't like this 2 weeks ago (if i remember the time right!). and i did tell you about the conversion not working many times too. in different ways too. when your own sample doesnt works, the wiki doesn't do anything. all the codes are untouched and all updated to now. the driver is yours too. and it is the same destroyed audio for resample too.

josef2600 commented 5 days ago

and if you are interested, in arduino-audio-driver the Utils\Logger.c has a mistake. it has set int LOGLEVEL_AUDIODRIVER = 2; so, it doesn't change the log level with AudioLogger::Info . can you fix it?

pschatzmann commented 5 days ago

I am not aware of any problems with the logger: just follow the instructions from the Readme

pschatzmann commented 5 days ago

My guess would be that kit.begin(cfg); might send some data already, but at this time you did not set up the convert is not set up.

Try to change the sequence and call kit begin last like in the provided example!

josef2600 commented 5 days ago

I am not aware of any problems with the logger: just follow the instructions from the Readme thank you. i know that, but i was wondering if it is possible to change AudioDriverLogLevel::Info with audio-tools too. since many library are in the work and all can be set with audio-tools

pschatzmann commented 5 days ago

Not really: the audio-drivers do not have any dependencies from the AudioTools, so they do not know anything about this project. However the integer values of the enums are consistent, so you could just cast to AudioDriverLogLevel

josef2600 commented 4 days ago

thank you. understood. for convert: i did this:

  auto cfg = kit.defaultConfig();
  cfg.sd_active = false;
  cfg.copyFrom(info);
  delay(100);
  convert.begin(16, 24);
  kit.begin(cfg);
  delay(100);
  a2dp_sink.start("AudioKit");

the sound did came-out but, it was cut. like 200ms with sound and 200ms no sound. changed it to this:

  auto cfg = kit.defaultConfig();
  cfg.sd_active = false;
  cfg.copyFrom(info);
  kit.begin(cfg);
  delay(100);
  convert.begin(16, 24);
  delay(100);
  a2dp_sink.start("AudioKit");

the same cutting sound. removing AudioLogger::instance().begin(Serial, AudioLogger::Info); has no effect. changing to 32bit like this:

AudioBoardStream kit(AudioKitEs8388V2);  //AudioBoard.h  static AudioBoard AudioKitEs8388V2{AudioDriverES8388, PinsAudioKitEs8388v2};
NumberFormatConverterStream convert(kit);
BluetoothA2DPSink a2dp_sink(convert);
AudioInfo info(44100, 2, 32);

void setup()
{
  Serial.begin(115200);
  delay(500);
  AudioLogger::instance().begin(Serial, AudioLogger::Info);
  delay(100);
  auto cfg = kit.defaultConfig(TX_MODE);
// or this: auto cfg = kit.defaultConfig();
  cfg.sd_active = false;
  cfg.copyFrom(info);
  delay(100);
  convert.begin(16, 32);
  kit.begin(cfg);
  delay(100);
  a2dp_sink.start("AudioKit");
  delay(100);
}
void loop()
{
  delay(100);
}

funny! this one works. why 24bit doesn't work? and it is load! high output volume. good.

i was looking at AudioStreamsConverter.h and i was seeing this int24_t . can it be the cause? it is used for 24bit codes.

pschatzmann commented 4 days ago

I think you just ignored the information about the log level: You just can't use log level Info with 24 or 32 bits w/o impacting the audio!

josef2600 commented 4 days ago

I think you just ignored the information about the log level: You just can't use log level Info with 24 or 32 bits w/o impacting the audio!

thanks, but, sorry i don't understand. where did log level Info came from? what i understood is that converting from 16 to 24 bit doesn't work, but from 16bit to 32bit works. why? did i do something wrong?

pschatzmann commented 4 days ago

Your

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

should be

AudioToolsLogger.begin(Serial, AudioToolsLogLevel::Warning);

e.g. as described here

josef2600 commented 4 days ago

i did that. removing AudioLogger::instance().begin(Serial, AudioLogger::Info); has no effect. even tested AudioLogger::instance().begin(Serial, AudioLogger::Error); too, no effect. i said 32bit works, but 24bit doesn't work correctly.

pschatzmann commented 4 days ago

Can you provide your sketch that is not working ?

josef2600 commented 4 days ago

#define AUDIOKIT_BOARD  7

#include "AudioTools.h"
#include "AudioTools/AudioLibs/A2DPStream.h" // install https://github.com/pschatzmann/ESP32-A2DP
#include "AudioTools/AudioLibs/AudioBoardStream.h" // install https://github.com/pschatzmann/arduino-audio-driver
#include "BluetoothA2DPSink.h"

#define the_out_bitratee  24

//BluetoothA2DPSink a2dp_sink;
AudioBoardStream kit(AudioKitEs8388V2);  //AudioBoard.h  static AudioBoard AudioKitEs8388V2{AudioDriverES8388, PinsAudioKitEs8388v2};
NumberFormatConverterStream convert(kit);
BluetoothA2DPSink a2dp_sink(convert);
AudioInfo info(44100, 2, the_out_bitratee);

void setup()
{
  Serial.begin(115200);
  delay(500);
  AudioLogger::instance().begin(Serial, AudioLogger::Error);
  delay(100);

  auto cfg = kit.defaultConfig();
  cfg.sd_active = false;
  cfg.copyFrom(info);
  delay(100);
  convert.begin(16, the_out_bitratee);
  kit.begin(cfg);
  delay(100);
  a2dp_sink.start("AudioKit");

  delay(100);
}

void loop()
{
  delay(100);
}

complete code. i have es8388 ver2.

pschatzmann commented 4 days ago

Not sure how to resolve this: the issue seems to be in the new ESP32 I2S driver. With the old 2.x versions 24 bits are working!

josef2600 commented 4 days ago

but i am using the board version 2.0.14 on Arduino. thank you. anyway, the 32bit works, but i don't know how es8388 handles the extra byte. since it is a 24bit DAC but it also accepts 32bit too. on another note, is there a code i can do for conversion in the Arduino itself? like using this:

void read_data_stream(const uint8_t *dataaa, uint32_t lengthh)
{

  // data is provided as array of int16
  uint32_t sample_count = lengthh / sizeof(uint16_t);
  uint16_t* data16 = (uint16_t*) dataaa;
  for (int j = 0; j < sample_count; j++)
  {
    // convert to 24 bit data
    uint32_t sample32 = static_cast<uint32_t>(data16[j]) << 8;
    // add your special logic here
    // output 24 bits to DAC
    kit.write((uint8_t*)&sample32, 4);
  }

}

pschatzmann commented 4 days ago

I don't think so, but you can try: the current documented logic should be static_cast(data16[j]) << 16. It expects a 32 bit integer and just ignores the last 8 bits. At least that's what it used to be.

I think we should better try to figure out the correct settings in the new IDF I2S API. If you review my implementation, you might find what I am overlooking....

pschatzmann commented 4 days ago

I was testing with 2.0.17 and there it was working...

marecl commented 4 days ago

I'm going to drop in to say that NumberFormatConverterStream absolutely corrupts my stream.
I need 32-bit output. Without converter, forcing I2S 32-bit output yields "some" sound by concatenating neighbouring samples, which is unusable (octave higher pitch, buffer underruns).
Converter makes the whole stream basically pinned to the extremes:

#include "config.h"
#include "AudioTools.h"
#include "BluetoothA2DPSinkQueued.h"

AudioInfo info(44100, 2, 32);
I2SStream i2s;
NumberFormatConverterStream convert(i2s);
static BluetoothA2DPSinkQueued a2dp_sink(convert);

void audio_init() {
  AudioLogger::instance().begin(Serial, AudioLogger::Debug);
  AudioLogger::instance().begin(Serial, AudioLogger::Warning);
  AudioLogger::instance().begin(Serial, AudioLogger::Error);
  AudioLogger::instance().begin(Serial, AudioLogger::Info);

  auto cfg = i2s.defaultConfig();
  cfg.copyFrom(info);
  cfg.pin_bck = BCK_PIN;
  cfg.pin_ws = LRCK_PIN;
  cfg.pin_data = DATA_PIN;
  cfg.i2s_format = I2S_PCM;

  convert.begin(16,32);

  i2s.begin(cfg);

  a2dp_sink.set_volume(3 * 128 / 4);  // 0-127
  a2dp_sink.set_on_connection_state_changed(connection_changed_cb);
  a2dp_sink.start(DEVICE_NAME, true);
}

obraz obraz Removing converter:

#include "config.h"
#include "AudioTools.h"
#include "BluetoothA2DPSinkQueued.h"

AudioInfo info(44100, 2, 32);
I2SStream i2s;
//NumberFormatConverterStream convert(i2s);
static BluetoothA2DPSinkQueued a2dp_sink(i2s);

void audio_init() {
  AudioLogger::instance().begin(Serial, AudioLogger::Debug);
  AudioLogger::instance().begin(Serial, AudioLogger::Warning);
  AudioLogger::instance().begin(Serial, AudioLogger::Error);
  AudioLogger::instance().begin(Serial, AudioLogger::Info);

  auto cfg = i2s.defaultConfig();
  cfg.copyFrom(info);
  cfg.pin_bck = BCK_PIN;
  cfg.pin_ws = LRCK_PIN;
  cfg.pin_data = DATA_PIN;
  cfg.i2s_format = I2S_PCM;
  i2s.begin(cfg);

  a2dp_sink.set_volume(3 * 128 / 4);  // 0-127
  a2dp_sink.start(DEVICE_NAME, true);
}

obraz obraz

I have a 16-bit 44.1kHz input, so in theory it should work. I barely changed code that worked before all updates, but can't really turn back now.
[edit] Downgraded to 2.0.14 and 2.0.17 but that doesn't work for me either (Complaining about losing previous versions of software on git, kind of ironic). I've been using Queued32 unofficial fix for this, but now my hands are tied.
Unless I'm making a very simple error that I will realize after I post this issue, but as for now I'm stuck.

pschatzmann commented 4 days ago

Removing the converter and output with 32 bits is just plain wrong!

I just made another test with 32 bits and my audiokit and it is working perfectly with 32 bits also with the BluetoothA2DPSinkQueued class.

Finally I am not supporting the old and obsolete 2.014. We are currently on 3.0.7 with 3.1 coming out soon.

Not sure what made you select I2S_PCM. I have never used this and never seen a DAC which would require this, so you are on your own there.

marecl commented 4 days ago

Removing the converter and output with 32 bits is just plain wrong!

Hmmm. I guess you're right but at least I know my receiver is working :dancing_men:
I'm reviving a CD player and it accepts only PCM_LONG at 32bits 44.1k. But for some reason Espressif it was dropped for ESP32 and now I'm in a pickle. I remember using it a year or two ago with no problems with the same WROOM board.
Anyway I decided to update everything to git and... sorry to bother. False alarm. Arduino library versions are far back...

A-KL commented 1 day ago

Maybe you need to set another I2S format

Just got my second DAC working!

While testing it with raspberry pi and volumio I accidentally touched the ground terminal and some of the i2s input pins which gave a perfectly clear sound.

ES9018-based purple TeraDaK boards do require MCLK input to be grounded! Hope it will be useful for someone 😃

Otherwise: upscaling to 32bit with I2S_STD_FORMAT and even debug level set to AudioLogger::Info works like a charm now. Although I did hear some hiccups every 1-2 second(s) while testing it before, so definitely not the ideal setting.

josef2600 commented 18 hours ago

if i may, we had a discussion about it. phil is away for few days. but we both believe this is not the code from his libraries. i have don extensive testings with few different DACs and settings. at least i believe it is do to messed up job of the I2S driver in Arduino. it is not sending the correct data to output. so please kip it in mind. @A-KL : good job, but as far as i know, ES9018 is not included in the driver library from pschatzmann. and you are driving it by default setting. it has an I2C protocol for setting it up and its own driver. i do not have it, but i would think it is an excellent DAC. we need some where to put the fixed issues, not make a new issue. the discussions are too large.

A-KL commented 15 hours ago

Thanks, @josef2600. A while ago, I wrote some simple code to control ES9018 over I2C. It's nice to be able to change options like I2S sample size or volume level/gain. Unfortunately, most ES9018-based boards I've seen - don't expose I2C port. Some of them have pull-up resistors so it's possible to solder wires directly to a board. Otherwise, I agree - great DAC!

Btw, I think we can close the issue. It looks like all issues related to the library were resolved.

josef2600 commented 12 hours ago

yes! i couldn't close it! but you did it. thanks for that. i probably put a link when i am don with this i2s thingy. or at least i get some where. but right now i can tell you the setting do not work correctly. it is not from here. for example, i have a DAC that works with MSB first , but with that setting, i only get garbage. but if i swipe the high and low bytes (changing it to LSB first) it works! i mean that is so bad. and i didn't use all of pschatzmann codes. i send the data from BLT directly to I2S hardware with API of Arduino. and it was that. this is messed up. their driver doesn't work correctly. now go tell them if you can ! they cant even test it. at best they say because it is working with that setting, you are wrong! are you kidding me? it's a Japanese DAC. do they make a wrong datasheet or DAC ? but this is china for us!

pschatzmann commented 11 hours ago

If you found some bugs, please open an issue at the corresponding Espressif Github Poject.

I am currently still struggeling to understand what happening with the 24 bits and why it is working with a simple sine 24 bit test and a sine 16 bit test converted to 24 bits, but not with the 16 bits from A2DP converted to 24 bits.

josef2600 commented 10 hours ago

hey! you are back! shouldn't you be off the monitor or work? it is not just that. i am testing a large amount of codes and settings. but for now i am working on clocks of i2s. well, i think this may be related to ESP32-A2DP. basically it doesn't allow me (or the coder!) to change anything. it always gets back to its setting. lets take the program apart. in simple ways: we have input. for example Bluetooth. it should be a simple reading data from the source. it should not control the output. but it does. it is gloude to i2s0 with all configuration protected. for example, in BluetoothA2DPOutput.h we have this:

 protected:
  i2s_config_t i2s_config;
  i2s_pin_config_t pin_config;
  i2s_channel_t i2s_channels = I2S_CHANNEL_STEREO;
  i2s_port_t i2s_port = I2S_NUM_0;

or :

 protected:
  BluetoothA2DPOutputAudioTools out_tools;
  BluetoothA2DPOutputLegacy out_legacy;

this configuration closes every setting to be changed. for example, i make this:

#include "AudioTools.h"
#include "BluetoothA2DPSink.h"
#include <driver/i2s.h>

const uint8_t I2S_SCK = 5;       // Audio data bit clock
const uint8_t I2S_WS = 25;       // Audio data left and right clock
const uint8_t I2S_SDOUT = 26;    // ESP32 audio data output (to speakers)

#define the_i2s_sample_rate 192000  // 192 kHz sample rate

BluetoothA2DPSink a2dp_sink;

void audio_data_callback(const uint8_t* data, uint32_t len) {
    size_t written;
    i2s_write(I2S_NUM_0, data, len, &written, portMAX_DELAY);
}

void setup() {
  Serial.begin(115200);
  delay(500);

  // Initialize BluetoothA2DPSink first
  a2dp_sink.start("ESP32 Music");
  delay(100);

  // Uninstall any existing I2S driver that might have been set up by BluetoothA2DPSink
  i2s_driver_uninstall(I2S_NUM_0);

  // I2S configuration
  i2s_config_t i2s_config = {
    .mode = (i2s_mode_t)(I2S_MODE_MASTER | I2S_MODE_TX),
    .sample_rate = the_i2s_sample_rate,
    .bits_per_sample = I2S_BITS_PER_SAMPLE_16BIT,
    .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT,
    .communication_format = I2S_COMM_FORMAT_STAND_MSB,
    .intr_alloc_flags = ESP_INTR_FLAG_LEVEL1,
    .dma_buf_count = 8,
    .dma_buf_len = 512,
    .use_apll = true,
    .tx_desc_auto_clear = true,
    .fixed_mclk = 0
  };

  // I2S pin configuration
  i2s_pin_config_t pin_config = {
    .bck_io_num = (gpio_num_t)I2S_SCK,
    .ws_io_num = (gpio_num_t)I2S_WS,
    .data_out_num = (gpio_num_t)I2S_SDOUT,
    .data_in_num = I2S_PIN_NO_CHANGE
  };

  // Install and start I2S driver
  esp_err_t err = i2s_driver_install(I2S_NUM_0, &i2s_config, 0, NULL);
  if (err != ESP_OK) {
    Serial.print("Error installing I2S driver: ");
    Serial.println(err);
  } else {
    Serial.println("I2S driver installed successfully.");
  }

  err = i2s_set_pin(I2S_NUM_0, &pin_config);
  if (err != ESP_OK) {
    Serial.print("Error setting I2S pins: ");
    Serial.println(err);
  } else {
    Serial.println("I2S pins set successfully.");
  }

  err = i2s_set_clk(I2S_NUM_0, the_i2s_sample_rate, I2S_BITS_PER_SAMPLE_16BIT, I2S_CHANNEL_STEREO);
  if (err != ESP_OK) {
    Serial.print("Error setting I2S clock: ");
    Serial.println(err);
  } else {
    Serial.println("I2S clock set successfully.");
  }

  delay(100);
  a2dp_sink.set_stream_reader(audio_data_callback); // Use callback to handle data manually
}

void loop() {
  delay(500);
}

i am uninstalling I2S_NUM_0after a2dp_sink.start then install I2S_NUM_0 with my own settings. for example , in here, it should make the clock of around 6.18 mhz. it does that, but as soon as it connects to a Bluetooth, all of my settings are tron out and it redefine all of its own settings and it gets back to 1.41 mhz. so nobody can change anything. it was a simple example. but i am working on it. about the Espressif Github , well, i don't like it. as far as i can, either i find a solution myself, or i give up and leave it. its a long story. and that is the main reason i don't work with core boards v3 or anythings except 2.0.14 . it wasted so much time of me. it cant even handle a complex analog read. and when i told them when i work with core 1 and multitasking, it crashes, they say it works for ours. how did they tested ? they put adcread in the loop function. what a great team. but as i explained before, it is working incorrectly with MSB and LSB and other stuff too. i prefer to do all of my testing, then when i fail, trough all of my works to the trash and then, i dont know. i know i have never encountered any of these problem with other microcontrollers. they never change their base drivers. right now, their core Ethernet drivers are so full of errors. they have removed at least 30% of its functionality and still has many errors and doesn't work correctly. i know a couple of their coders, and they are really bad at coding. sorry, i am ranting here.

pschatzmann commented 10 hours ago

Why are you wasting your time with this old, obsolete release ? I think the best way forward is to always use the latest release and file a bug report when something is not working! If you stick with 2.0.14, nobody will care because some issues have been fixed on subsequent releases.

Since the I2S driver has completly changed as well, it is also not worth to invest any time in these old drivers

pschatzmann commented 10 hours ago

ps. You can deactivate all I2S by setting A2DP_I2S_AUDIOTOOLS and A2DP_LEGACY_I2S_SUPPORT to false in config.h In your old release it might be a bit messy because I wanted to be backward compatible. If you would use a recent ESP32 release, I am sure that there is no impact to I2S if BluetoothA2DPSink is not connected to an output class.

pschatzmann / arduino-audio-tools