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

Corrupted sound after extending sample size to 32bit #1776

Open
1 task done
A-KL opened this issue Nov 5, 2024 · 13 comments
Open
1 task done

Corrupted sound after extending sample size to 32bit #1776

A-KL opened this issue Nov 5, 2024 · 13 comments

Comments

@A-KL
Copy link

A-KL commented Nov 5, 2024

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

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

  • I confirm I have checked existing issues, discussions and online documentation
@pschatzmann
Copy link
Owner

pschatzmann commented Nov 5, 2024

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
Copy link
Author

A-KL commented Nov 5, 2024

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
Copy link
Owner

pschatzmann commented Nov 5, 2024

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
Copy link
Owner

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

@pschatzmann pschatzmann reopened this Nov 5, 2024
@pschatzmann
Copy link
Owner

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

@A-KL
Copy link
Author

A-KL commented Nov 5, 2024

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
Copy link
Owner

pschatzmann commented Nov 5, 2024

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
Copy link
Author

A-KL commented Nov 5, 2024

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
Copy link
Owner

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

@A-KL
Copy link
Author

A-KL commented Nov 5, 2024

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
Copy link
Owner

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
Copy link
Author

A-KL commented Nov 5, 2024

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
Copy link
Owner

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())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants