-
Notifications
You must be signed in to change notification settings - Fork 237
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
No AudioOut sounds with ESP-IDF v4.4 #881
Comments
Thanks for the report. We'll take a look. This wouldn't be the first time that an ESP-IDF update caused audio problems, alas. |
Based on testing from @mkellner, it appears that I²S audio works, for example on the M5Atom Echo. The failures occur when using the internal ESP32 DAC, such as on the M5Stack and M5Stack Core2. |
This one is stubborn. There are no errors reported by the
It appears that at least one other developer has encountered this problem with ESP-IDF 4.4, issue #6470 in the ESP-IDF repository. |
Hi @phoddie @meganetaaan, Sorry for the late reply, we have looked into this issue, it cause by the data bit shift which is introduced by There will be docs or force setting the communication mode to MSB. |
@L-KAYA – thank you for the reply. We'll give that a try. |
@L-KAYA – Unfortunately, setting the i2s_config_t i2s_config = {
.mode = I2S_MODE_MASTER | I2S_MODE_TX | I2S_MODE_DAC_BUILT_IN,
.sample_rate = out->sampleRate,
.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 = 0,
.dma_buf_count = 2,
.dma_buf_len = sizeof(out->buffer) / 2,
.use_apll = false
}; The audio played is still just a few pops. The 3rd point in the Problem Description for espressif/esp-idf#8634 suggests that the problem isn't just the format, as the timing of the buffer writes is much faster than expected.
|
Digging further. I found another related issue (espressif/esp-idf#8326) and its fix merged into the release/v4.4 branch. Since moddable uses esp-idf from the released tag (not the release branch), this fix is not yet available. I tried the latest commit in the release/v4.4 branch at my end. The sound did play, but twice as high and faster than it should. According to git blame, it appears to be broken again in the commit (espressif/esp-idf@15e8601) that was applied afterwards. I followed the first issue's fix commit (espressif/esp-idf@19f4eca) and changed clock calculate function to below. And now the correct playback speed is achieved. #if SOC_I2S_SUPPORTS_ADC || SOC_I2S_SUPPORTS_DAC
/**
* @brief I2S calculate clock for built-in ADC/DAC mode
*
* @param i2s_num I2S device number
* @param clk_cfg Struct to restore clock confiuration
* @return
* - ESP_OK Get clock success
* - ESP_ERR_INVALID_ARG Invalid args
*/
static esp_err_t i2s_calculate_adc_dac_clock(int i2s_num, i2s_hal_clock_cfg_t *clk_cfg)
{
ESP_RETURN_ON_FALSE(clk_cfg, ESP_ERR_INVALID_ARG, TAG, "input clk_cfg is NULL");
ESP_RETURN_ON_FALSE(p_i2s[i2s_num]->hal_cfg.mode & (I2S_MODE_DAC_BUILT_IN | I2S_MODE_ADC_BUILT_IN), ESP_ERR_INVALID_ARG, TAG, "current mode is not built-in ADC/DAC");
/* Set I2S bit clock */
clk_cfg->bclk = p_i2s[i2s_num]->hal_cfg.sample_rate * I2S_LL_AD_BCK_FACTOR;
/* Set I2S bit clock default division */
clk_cfg->bclk_div = p_i2s[i2s_num]->hal_cfg.chan_bits;
/* If fixed_mclk and use_apll are set, use fixed_mclk as mclk frequency, otherwise calculate by mclk = sample_rate * multiple */
clk_cfg->mclk = (p_i2s[i2s_num]->use_apll && p_i2s[i2s_num]->fixed_mclk) ?
p_i2s[i2s_num]->fixed_mclk : clk_cfg->bclk * clk_cfg->bclk_div;
/* Calculate bclk_div = mclk / bclk */
// clk_cfg->bclk_div = clk_cfg->mclk / clk_cfg->bclk;
clk_cfg->bclk_div = I2S_LL_AD_BCK_FACTOR * 16;
/* Get I2S system clock by config source clock */
clk_cfg->sclk = i2s_config_source_clock(i2s_num, p_i2s[i2s_num]->use_apll, clk_cfg->mclk);
/* Get I2S master clock rough division, later will calculate the fine division parameters in HAL */
clk_cfg->mclk_div = clk_cfg->sclk / clk_cfg->mclk;
/* Check if the configuration is correct */
ESP_RETURN_ON_FALSE(clk_cfg->mclk <= clk_cfg->sclk, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large");
return ESP_OK;
}
#endif // SOC_I2S_SUPPORTS_ADC || SOC_I2S_SUPPORTS_DAC Still I'm not sure it is a degrade on the ESP-IDF side or if it can be corrected with arguments given to i2s_config, etc. |
@meganetaaan – Wow. Very impressive investigation! Thank you. I hope this will be enough to allow @L-KAYA to recommend a solution. |
@meganetaaan Thanks a lot for investigating on this issue, and really sorry for the late reply @phoddie , I'm busy on other tasks recently. I have tested on it, for the issue that twice faster as it sound, it might be caused by the incorrect data format in the sending buffer or the incorrect The format For the case that needs to transmit same data on two channels with the setting sample rate, the Therefore, setting the Summarily, the driver before v4.4 used a lot of fixed configurations while ignoring the configurations given by user, and unfortunately, the ignored configurations in the example is not correct, it is actually a breaking change I think. |
@L-KAYA – Thank you for the additional details. I understand what you are saying and agree that it could explain the 2x speed difference. However, there is a separate problem to address first -- because I have no audio, not audio at the wrong speed. As @meganetaaan notes, we are using the
If I am following what @meganetaaan said, I need to switch to some other branch / tag / commit to get a version of I²S support that can work, then I can make the change to deal with mono/stereo correctly. But... what is the branch / tag / commit to use? |
@phoddie You can try to use the following command to clone the latest version
or just switch to
The branch |
We'e committed changes that work on the M5Stack Fire with the @meganetaaan, it would be much appreciated if you could confirm that this works for you as well. If it does, we will need to update the Moddable SDK instructions for ESP-IDF set-up, though I'm a little uncomfortable not having a tag to reference (we have had problems before where the ESP-IDF changed in an incompatible way, causing problems for developers before we noticed). @L-KAYA, thank you very much for your patient help with this!! |
I tried the latest commit of out.mp4Left: the last working commit, Right: current commit
+1. These kind of breaking changes is likely to happen in the future. If tags are not available, I would suggest to rely on a specific commit rather than a branch. |
@meganetaaan – good observation about the pitch difference! It does seem to be high by about two half-steps. I was so happy that sound played at all, I completely overlooked that. I have two suggestions:
If you agree, I will move ahead with those. Please let me know. |
Agreed with your suggestions 🙆♀️. Since we've got the audio playback back, we can close this issue (and open new one at esp-idf repos). |
Perfect. Thank you.
Me too. I'm looking forward to see what comes next. |
I noticed the latest v4.4.1 release includes the fix described above. I think we can switch to this version. It is more reasonable than pointing to a specific commit. |
Build environment: Linux
Target device: M5Stack Basic/ M5Stack CORE2
Description
With M5Stack and the latest ModdableSDK public build, no sound can be played through the AudioOut class, leaving a slight noise at the beginning.
Steps to Reproduce
piu/sounds
example app usingmcconfig -m -p esp32/m5stack
.Other information
After a little digging:
AudioOut.Callback
is successfully invoked.AudioOut.Tone
also cannot be played.Then I guessed there is a regression or breaking change in i2s features of esp-idf but couldn't go further.
The text was updated successfully, but these errors were encountered: