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

Crackling sound on i2s with internal DAC since 2.0.10 #8467

Closed
1 task done
harbaum opened this issue Jul 27, 2023 · 28 comments
Closed
1 task done

Crackling sound on i2s with internal DAC since 2.0.10 #8467

harbaum opened this issue Jul 27, 2023 · 28 comments
Assignees
Milestone

Comments

@harbaum
Copy link

harbaum commented Jul 27, 2023

Board

ESP32 dev board

Device Description

DevKitC

Hardware Configuration

Speaker on GPIO25

Version

latest master (checkout manually)

IDE Name

Arduino IDE

Operating System

Linux and Windows

Flash frequency

40Mhz

PSRAM enabled

no

Upload speed

921000

Description

This demo sketch used to play a rectangle wave form on GPIO25 it works on 2.0.6 to 2.0.9

Since 2.0.10 sound is distorted and has a crackling noise overlaid. Oscilloscope shows short spikes in the rectangle signal.

Sketch

#include "driver/i2s.h"

unsigned short snd_buffer[64];

void setup() {
    static const i2s_config_t i2s_config = {
    .mode = (i2s_mode_t)(I2S_MODE_MASTER | I2S_MODE_TX | I2S_MODE_DAC_BUILT_IN),
    .sample_rate = 24000,
    .bits_per_sample = I2S_BITS_PER_SAMPLE_16BIT,

    .channel_format = I2S_CHANNEL_FMT_ONLY_RIGHT,
    .intr_alloc_flags = 0,
    .dma_buf_count = 4,
    .dma_buf_len = 64,   // 64 samples
    .use_apll = true
  };
  i2s_driver_install(I2S_NUM_0, &i2s_config, 0, NULL);
  i2s_set_dac_mode(I2S_DAC_CHANNEL_RIGHT_EN);
}

void loop() {
  // generate square wave
  for(int i=0;i<32;i++)  snd_buffer[i] = 0x0000;
  for(int i=32;i<64;i++) snd_buffer[i] = 0x1000;

  size_t bytesOut = 0;
  i2s_write(I2S_NUM_0, snd_buffer, sizeof(snd_buffer), &bytesOut, 0);
  printf("BytesOut: %d\n", bytesOut);
}

Debug Message

Endless sequence of
BytesOut: 128
BytesOut: 0

No difference in output between 2.0.9 and 2.0.10

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@VojtechBartoska
Copy link
Contributor

@me-no-dev Please help with triage, thanks

@harbaum harbaum changed the title Crackling sound on i2c with internal DAC since 2.0.10 Crackling sound on i2s with internal DAC since 2.0.10 Jul 27, 2023
@SuGlider
Copy link
Collaborator

SuGlider commented Jul 27, 2023

@harbaum - DAC only supports 8 bits sample per channel. This is the problem.
Anyway, this is an IDF issue, not related to the Arduino Core directly.

@SuGlider
Copy link
Collaborator

You can try this:

    // adjust the samples to 8bit ch1 + 8bit ch2
    .bits_per_sample = I2S_BITS_PER_SAMPLE_16BIT,        // the DAC only uses 8 bits per channel
    .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT,        // stereo

or

    // adjust samples to 8bit single channel
    .bits_per_sample = I2S_BITS_PER_SAMPLE_8BIT,         // the DAC only uses 8 bits
    .channel_format = I2S_CHANNEL_FMT_ONLY_LEFT,         // mono

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

@SuGlider Thanks for the reply. I'll give this a try asap. However,

the example on this page:
https://docs.espressif.com/projects/esp-idf/en/v4.3.5/esp32/api-reference/peripherals/i2s.html

includes this line

.bits_per_sample = I2S_BITS_PER_SAMPLE_16BIT, /* the DAC module will only take the 8bits from MSB */

This was the reason why I chose 16 bit.

Later on the page states:

I2S_MODE_DAC_BUILT_IN = 16
Output I2S data to built-in DAC, no matter the data format is 16bit or 32 bit, the DAC module will only take the 8bits from MSB

For some reason all these comments are gone in later ESP-IDF docs.

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

There's probably a directly relationship between arduino ESP32 board support versions (2.0.xx) and the ESP-IDF version (4.x/5.x).

This broke from 2.0.9 to 2.0.10. Where can I see what underlying ESP-IDF versions these two use?

Edit: 2.0.9 is based on ESP-IDF 4.4.4, 2.0.10 and 2.0.11 are based on 4.4.5

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

Some more investigation. The stereo version also crackles. So does only right and only left.

On the oscilloscope i see short spikes of the intended volume/voltage but not nearly as long as expected.

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

Interesting fact: I had left and right mixed in channel_format and set_dac_mode. That gave a sound on previous versions, while it IMHO shouldn't.

With 2.0.11 it only works with left/left or right/right. But still there's significant noise, which there isn't with 2.0.9 a and earlier. The example (now with right/right) gives a rectangle signal but with noticeable crackling noise. While the cracks are not extremely dominant in the rectangle version, they are very prominent in a real audio example.

Still, the example above produces noticeable crackling noises.

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

You can try this:

    .bits_per_sample = I2S_BITS_PER_SAMPLE_8BIT,         // the DAC only uses 8 bits
    .channel_format = I2S_CHANNEL_FMT_ONLY_LEFT,         // mono

Switching to 8 bits doesn't make a difference.

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

IMG_20230728_144010.jpg

The distortions seen on the oscilloscope.

@SuGlider
Copy link
Collaborator

SuGlider commented Jul 28, 2023

@harbaum - I've found what solves the issue.

Please use .use_apll = false instead of true.
I don't know exactly what has changed from the IDF 4.4 precompiled libraries used in the Arduino Core 2.0.9 to the Arduino Core 2.0.10.

It seems that it's nothing that has chaged in the Arduino Layer... maybe something changed in the IDF side.
I'll try to quickly see if there is something in the IDF github that may justify this finding.

@SuGlider
Copy link
Collaborator

I couldn't see any change in https://github.com/espressif/esp-idf/blob/release/v4.4/components/driver/i2s.c that would justify such change. Maybe something changed in the components used to calculate and set the I2S source clock when it uses APLL.

Thanks for reporting! You may want to open an issue within IDF github to report this APLL issue.

@SuGlider SuGlider self-assigned this Jul 28, 2023
@SuGlider SuGlider added Type: Question Only question Status: Solved and removed Status: Awaiting triage Issue is waiting for triage labels Jul 28, 2023
@SuGlider
Copy link
Collaborator

@VojtechBartoska - You may want to report it directly to the I2S IDF team. Thanks!

@harbaum
Copy link
Author

harbaum commented Jul 28, 2023

Please use .use_apll = false instead of true.

This seems to solve the issue for 24kHz and the resulting frequency in my demo sketch here is indeed, 24000/64 = 375Hz

But Galagino (https://github.com/harbaum/galagino) also uses 11765 Hz (as that's the frequency the Donkey Kong Arcade based its audio on). This should be 11765/64 = 183Hz. And with appl=true I really get a (crackling) 183 Hz signal. With apll=false I get 462 Hz which in turn would mean that the sample rate is 30kHz rather than the requested 11765 Hz.

Actually with apll=false I get totally odd frequencies with the above example just by changing the sample_rate. E.g.:

24000 results in 375Hz which indeed is a sample rate of 24000 Hz
20000 results in 312Hz which indeed is a sample rate of 20000 Hz
12000 results in 486Hz which actually is a sample rate of 31000 Hz
10000 results in 320Hz which actually is a sample rate of 20480 Hz
8000 results in 691Hz which actually is a sample rate of 44224Hz
6000 results in 1200Hz which actually is a sample rate of 76800Hz

So with apll=false the sample rates are messed up somehow ...

How is the sample_rate supposed to work with apll = false?

@SuGlider
Copy link
Collaborator

SuGlider commented Jul 29, 2023

I got some better results with .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT, instead of I2S_CHANNEL_FMT_ONLY_RIGHT
Using .use_apll = true

Example with 6,400 samples per second in a 64 samples per cycle (Hz) running OK.

    static const i2s_config_t i2s_config = {
    .mode = (i2s_mode_t)(I2S_MODE_MASTER | I2S_MODE_TX | I2S_MODE_DAC_BUILT_IN),
    .sample_rate = 6400,
    .bits_per_sample = I2S_BITS_PER_SAMPLE_16BIT,

    .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT, //I2S_CHANNEL_FMT_ONLY_RIGHT,
    .intr_alloc_flags = 0,
    .dma_buf_count = 4,
    .dma_buf_len = 64,   // 64 samples
    .use_apll = true
  };

Please also try it and let me know.

@harbaum
Copy link
Author

harbaum commented Jul 29, 2023

I got some better results with .channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT, instead of I2S_CHANNEL_FMT_ONLY_RIGHT Using .use_apll = true
...
Please also try it and let me know.

It's indeed better, but still crackling. The rectangle wave seems to hide the distortions somewhat. They become much more prominent with real life audio or e.g. with a sine wave generated like this:

for(int i=0;i<64;i++)
     snd_buffer[i] = (int)(0x8000 + 0x4000 * sin(i*M_PI/32))&0xff00;

@harbaum
Copy link
Author

harbaum commented Jul 29, 2023

Another interesting observation:

This should be a saw tooth:

for(int i=0;i<64;i++)
    snd_buffer[i] = (i&15)<<8;

But on the scope it looks like this:

image

It looks correct if I swap every second byte like so:

for(int i=0;i<64;i++)
    snd_buffer[i^1] = (i&15)<<8;

image

@harbaum
Copy link
Author

harbaum commented Jul 29, 2023

Arduino ESP32 version 2.0.9 was ESP-IDF 4.4.4 and 2.0.10 uses 4.4.5

So this problem may have been introduced in ESP-IDF 4.4.5

@SuGlider
Copy link
Collaborator

So this problem may have been introduced in ESP-IDF 4.4.5

It is very possible. I know that I2S has been completely refactored for the IDF 5.x

@SuGlider
Copy link
Collaborator

for(int i=0;i<64;i++) snd_buffer[i^1] = (i&15)<<8;

I have not put too much thinkning here, but we have to remember that ESP32 is little endian.
The best way to set it may be with 2 bytes and a OR operation.

@VojtechBartoska VojtechBartoska moved this to Under investigation in Arduino ESP32 Core Project Roadmap Aug 16, 2023
@VojtechBartoska VojtechBartoska added Status: In Progress Issue is in progress and removed Status: Solved labels Aug 16, 2023
@VojtechBartoska VojtechBartoska moved this from Under investigation to In Progress in Arduino ESP32 Core Project Roadmap Aug 21, 2023
@lucasssvaz lucasssvaz self-assigned this Aug 22, 2023
@lucasssvaz
Copy link
Collaborator

lucasssvaz commented Aug 22, 2023

I'm taking a look at the changes from IDF 4.4.4 to 4.4.5. The main changes to I2S are the RX/TX channel format configuration for ESP32/ESP32-S2 and changes to the endianess when time division multiplexing is supported.

@SuGlider @harbaum Is this issue confirmed to be only on ESP32 or wasn't it tested in other SOCs ?

@harbaum
Copy link
Author

harbaum commented Aug 22, 2023

@SuGlider The ESP32 is imho the only variant supporting the DAC via I2S. So I don't think this can even happen on a different SoC.

@lucasssvaz
Copy link
Collaborator

I'll to some testing and report back my findings

@lucasssvaz
Copy link
Collaborator

@harbaum I reverted the I2S changes. I don't have a oscilloscope with me, could you test if the crackling still happens ?
This binary uses a sample rate of 11765 with APLL enabled.

i2stest.ino.esp32.bin.zip

If the crackling still happens it might be related to changes in the ADC or clocking.

@harbaum
Copy link
Author

harbaum commented Aug 26, 2023

Thanks a lot. I'll try asap and report back.

@harbaum
Copy link
Author

harbaum commented Aug 26, 2023

I just found that I still have the test setup at my bench.

So yes, the binary you sent me gives a 186Hz square wave and I hear no crackling at all. On the scope, there are slight distortions, but that may be due to my rather simple setup.

@VojtechBartoska
Copy link
Contributor

Update:

  • @lucasssvaz share the commit which broke this in ESP-IDF

2 options:

  • first update the I2S lib with reverted changes manually
  • add the patch to Arduino Lib builder

@lucasssvaz
Copy link
Collaborator

lucasssvaz commented Aug 28, 2023

This is the IDF commit that introduced this issue:
espressif/esp-idf@73ca054

Note that reverting this commit might break I2S microphones.

@lucasssvaz lucasssvaz added Status: Review needed Issue or PR is awaiting review and removed Status: In Progress Issue is in progress labels Aug 29, 2023
@lucasssvaz lucasssvaz added Type: Bug 🐛 All bugs and removed Status: Review needed Issue or PR is awaiting review labels Aug 30, 2023
@VojtechBartoska
Copy link
Contributor

closed by #8583

@lucasssvaz lucasssvaz added Status: Solved and removed Type: Question Only question labels Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

4 participants