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

I2S APLL Clock is inaccurate at 44100Hz 16-bit 2ch (IDFGH-1058) #3380

Closed
redchenjs opened this issue Apr 28, 2019 · 12 comments
Closed

I2S APLL Clock is inaccurate at 44100Hz 16-bit 2ch (IDFGH-1058) #3380

redchenjs opened this issue Apr 28, 2019 · 12 comments
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@redchenjs
Copy link
Contributor

redchenjs commented Apr 28, 2019

Environment

  • Development Kit: Widora AIR v6.0
  • Module or chip used: ESP32 Rev.1
  • IDF version: v3.2
  • Build System: CMake
  • Compiler version: 1.22.0-80-g6c4433a
  • Operating System: Linux
  • Power Supply: USB

Problem Description

When the i2s APLL clock is set to 44100Hz 16-bit 2ch, the real rate is 43945.238, which is inaccurate and causes BT_APPL: Pkt dropped problem when using A2DP streaming.

Expected Behavior

Real rate should be 44099.xxx or 44100.xxx

Actual Behavior

Real rate is 43945.238(~155Hz Error)

Steps to repropduce

  1. Get the a2dp_sink example from esp-idf v3.2.
  2. Add .use_apll = 1, to i2s_config to enable APLL.
  3. Build the example and flash it to esp32.
  4. Connect to esp32 with your phone and play some music.

Code to reproduce this issue

The a2dp_sink example from esp-idf v3.2

Debug Logs

I (16536) I2S: APLL: Req RATE: 44100, real rate: 43945.238, BITS: 16, CLKM: 1, BCK_M: 8, MCLK: 11249981.000, SCLK: 1406247.625000, diva: 1, divb: 0
......
I (81856) BT_AV: Audio packet count 3800
I (83316) BT_AV: Audio packet count 3900
I (84766) BT_AV: Audio packet count 4000
I (86226) BT_AV: Audio packet count 4100
I (87686) BT_AV: Audio packet count 4200
W (88266) BT_APPL: Pkt dropped

I (89136) BT_AV: Audio packet count 4300
I (90596) BT_AV: Audio packet count 4400
I (92056) BT_AV: Audio packet count 4500
I (93506) BT_AV: Audio packet count 4600
W (93656) BT_APPL: Pkt dropped

I (94966) BT_AV: Audio packet count 4700
I (96416) BT_AV: Audio packet count 4800
W (97196) BT_APPL: Pkt dropped

I (97876) BT_AV: Audio packet count 4900
I (99336) BT_AV: Audio packet count 5000
I (100786) BT_AV: Audio packet count 5100
W (101896) BT_APPL: Pkt dropped

I (102246) BT_AV: Audio packet count 5200
I (103706) BT_AV: Audio packet count 5300
W (105016) BT_APPL: Pkt dropped
@github-actions github-actions bot changed the title I2S APLL Clock is inaccurate at 44100Hz 16-bit 2ch I2S APLL Clock is inaccurate at 44100Hz 16-bit 2ch (IDFGH-1058) Apr 28, 2019
@negativekelvin
Copy link
Contributor

negativekelvin commented Apr 28, 2019

If apll parameters are
sdm2 = 5
sdm1 = 8
sdm0 = 28
odiv = 0

then fout = 40000000 * (4 + 5 + 8/256 + 28/65536)) = 361267089.84375
apll_freq = 361267089.84375 / (2 * (0 + 2)) = 90316772.4609375

If clkm_num = 8, a = 0, b = 1
clkm_freq = 90316772.4609375 / 8 = 11289596.5576171875

If bck_div_num = 256 / (16 * 2) = 8
bck_freq = 1411199.5697021484375

and sample rate = 1411199.5697021484375 / (16 * 2) = 44099.986553192138671875

so yeah the driver is not doing a good job calculating apll parameters. Unless you have a rev0 chip which can't do fractional apll factors.

If apll parameters are
sdm2 = 5
sdm1 = 78
sdm0 = 194
odiv = 0
clkm_num = 8, a = 11, b = 45

then sample rate = 44099.998602648629653807277628032

Another:
sdm2 = 8
sdm1 = 124
sdm0 = 126
odiv = 5
clkm_num = 3, a = 25, b = 4
sample rate = 44100.0000262131

SUGGESTION: optimized parameter values for common sample rates should be determined externally and hardcoded

@redchenjs
Copy link
Contributor Author

As #1393 mentioned, after 63ddae5 the apll_predefine values were removed, and before that, the driver did have a hardcoded APLL parameter table and the 44100Hz rate is right, so why the apll_predefine values were removed?

@redchenjs
Copy link
Contributor Author

redchenjs commented Apr 30, 2019

Now my workaround is:

if (rate == 44100) {
    rtc_clk_apll_enable(1, 15, 8, 5, 6);
    ESP_LOGW(TAG, "enable workaround for i2s apll clock at 44100Hz 16-bit 2ch");
} else {
    i2s_set_sample_rates(I2S_NUM_0, rate);
}

It did help, there is no BT_APPL: Pkt dropped problem anymore.
You need to add #include "soc/rtc.h" to your source to get the rtc_clk_apll_enable() function:

void rtc_clk_apll_enable(bool enable, uint32_t sdm0, uint32_t sdm1, uint32_t sdm2, uint32_t o_div)

The hardcoded parameter values are from previous 63ddae5 removed apll_predefine table:

/**	
 * @brief Pre define APLL parameters, save compute time	
 *        | bits_per_sample | rate | sdm0 | sdm1 | sdm2 | odir	
 */	
static const int apll_predefine[][6] = {	
    {16, 11025, 38,  80,  5, 31},	
    {16, 16000, 147, 107, 5, 21},	
    {16, 22050, 130, 152, 5, 15},	
    {16, 32000, 129, 212, 5, 10},	
    {16, 44100, 15,  8,   5, 6},	
    {16, 48000, 136, 212, 5, 6},	
    {16, 96000, 143, 212, 5, 2},	
    {0,  0,     0,   0,   0, 0}	
};

@negativekelvin Thanks for your reply! I tried to use these values you calculated, but it seems that they won't work unless the i2s_set_clk() function were modified.

@steenou
Copy link

steenou commented May 3, 2019

Have you experienced glitches in the output like I have been experiencing? #3407 I've been seeing zeroes in the SD output which is audible as clicking and cracking

@pad52
Copy link

pad52 commented May 30, 2019

Same issue here, still not resolved: #2634

@ajita02
Copy link

ajita02 commented Jun 7, 2019

Try the attached patch. It has solved the inaccurate apll clock rate issue. The patch is w.r.t. latest IDF i.e. 4.0. Kindly help to share the result.
0001-driver-i2s-fix-apll_clock_rate-for-different-sample-.zip

@fknrdcls
Copy link

fknrdcls commented Jun 7, 2019

Just a couple of changes to the original calculation are needed as I already provided in #2634
#2634 (comment)

If I have any spare time away from work I'll clone modify and submit a pull request if possible.

There is no need for major changes my patch will get you as close as the PLL can do, which is usually 10-20ppm or something. (+/- the source clock ppm obviously) I have measured hundreds of arbitrary clock rates on MCLK with a high resolution frequency counter and have been using this in Audio projects for over a year now.

There is nothing wrong with the APLL it's a good PLL, this original code was erroneous in that it would overshoot the more significant sdm parameters because it would choose the value closer to the target even if it was slightly higher than the target value, forever setting the accuracy at the resolution of that coarser parameter. Choosing the closest value should only be done on the least significant parameter.

I also have a cheap way to measure the input frequency if you're running I2S in slave mode if anyone's interested.

@ajita02
Copy link

ajita02 commented Jun 7, 2019

Try the attached patch. It has solved the inaccurate apll clock rate issue. The patch is w.r.t. latest IDF i.e. 4.0. Kindly help to share the result.
0001-driver-i2s-fix-apll_clock_rate-for-different-sample-.zip

The attached patch give is percentage difference of 0.001.

@fknrdcls
Copy link

fknrdcls commented Jun 7, 2019

Yeah no the APLL can do better than that with this fix:
From: #2634 (comment)

For example:

I (712) I2S: APLL: Req RATE: 48000, real rate: 47999.992, BITS: 32, CLKM: 1, BCK_M: 8, MCLK: >24575996.000, SCLK: 3071999.500000, diva: 1, divb: 0

MCLK is off by 4 parts in 24.576 Million
0.00001627604167%

So I guess I was wrong it's usually off less than 10 parts per million.

I'll say it again: It's a good proper Audio PLL, and it is much better than esspresif is apparently OK with.

The patch is simple, it was just a brain fart by the original developer, what is disappointing is that it STILL HASN'T BEEN FIXED.

@fknrdcls
Copy link

fknrdcls commented Jun 7, 2019

Also still got that MP3 encoder if anyone's interested. Otherwise I have work to do.
48Khz 16bit Stereo 256kbps 37% CPU overall.

redchenjs added a commit to redchenjs/esp-idf that referenced this issue Jun 18, 2019
redchenjs added a commit to redchenjs/esp-idf that referenced this issue Jun 18, 2019
@redchenjs
Copy link
Contributor Author

redchenjs commented Jun 18, 2019

@ajita02 @fknrdcls Hi, I made a comparison between these two patches. From the results, they are all very accurate. So it's time to submit them.
@ajita02 's patch:

I (524) I2S: APLL: Req RATE: 10675, real rate: 10674.993, BITS: 16, ERROR: 0.000064%
I (537) I2S: APLL: Req RATE: 10675, real rate: 10674.993, BITS: 24, ERROR: 0.000067%
I (552) I2S: APLL: Req RATE: 10675, real rate: 10674.988, BITS: 32, ERROR: 0.000110%

I (568) I2S: APLL: Req RATE: 11025, real rate: 11024.999, BITS: 16, ERROR: 0.000009%
I (583) I2S: APLL: Req RATE: 11025, real rate: 11024.996, BITS: 24, ERROR: 0.000035%
I (599) I2S: APLL: Req RATE: 11025, real rate: 11024.990, BITS: 32, ERROR: 0.000089%

I (614) I2S: APLL: Req RATE: 16000, real rate: 15999.993, BITS: 16, ERROR: 0.000043%
I (630) I2S: APLL: Req RATE: 16000, real rate: 15999.992, BITS: 24, ERROR: 0.000049%
I (645) I2S: APLL: Req RATE: 16000, real rate: 15999.993, BITS: 32, ERROR: 0.000043%

I (661) I2S: APLL: Req RATE: 22050, real rate: 22049.980, BITS: 16, ERROR: 0.000089%
I (676) I2S: APLL: Req RATE: 22050, real rate: 22049.971, BITS: 24, ERROR: 0.000130%
I (692) I2S: APLL: Req RATE: 22050, real rate: 22049.980, BITS: 32, ERROR: 0.000089%

I (707) I2S: APLL: Req RATE: 32000, real rate: 31999.986, BITS: 16, ERROR: 0.000043%
I (722) I2S: APLL: Req RATE: 32000, real rate: 31999.984, BITS: 24, ERROR: 0.000049%
I (738) I2S: APLL: Req RATE: 32000, real rate: 31999.988, BITS: 32, ERROR: 0.000037%

I (753) I2S: APLL: Req RATE: 44100, real rate: 44099.961, BITS: 16, ERROR: 0.000089%
I (769) I2S: APLL: Req RATE: 44100, real rate: 44099.943, BITS: 24, ERROR: 0.000130%
I (784) I2S: APLL: Req RATE: 44100, real rate: 44099.988, BITS: 32, ERROR: 0.000027%

I (800) I2S: APLL: Req RATE: 48000, real rate: 47999.977, BITS: 16, ERROR: 0.000049%
I (815) I2S: APLL: Req RATE: 48000, real rate: 47999.953, BITS: 24, ERROR: 0.000098%
I (831) I2S: APLL: Req RATE: 48000, real rate: 47999.977, BITS: 32, ERROR: 0.000049%

I (846) I2S: APLL: Req RATE: 96000, real rate: 95999.953, BITS: 16, ERROR: 0.000049%
I (861) I2S: APLL: Req RATE: 96000, real rate: 95999.906, BITS: 24, ERROR: 0.000098%
I (877) I2S: APLL: Req RATE: 96000, real rate: 95999.984, BITS: 32, ERROR: 0.000016%

@fknrdcls 's patch:

I (527) I2S: APLL: Req RATE: 10675, real rate: 10675.000, BITS: 16, ERROR: 0.000000%
I (538) I2S: APLL: Req RATE: 10675, real rate: 10675.003, BITS: 24, ERROR: 0.000030%
I (554) I2S: APLL: Req RATE: 10675, real rate: 10675.006, BITS: 32, ERROR: 0.000055%

I (569) I2S: APLL: Req RATE: 11025, real rate: 11024.991, BITS: 16, ERROR: 0.000080%
I (585) I2S: APLL: Req RATE: 11025, real rate: 11024.992, BITS: 24, ERROR: 0.000071%
I (600) I2S: APLL: Req RATE: 11025, real rate: 11024.991, BITS: 32, ERROR: 0.000080%

I (616) I2S: APLL: Req RATE: 16000, real rate: 15999.986, BITS: 16, ERROR: 0.000085%
I (631) I2S: APLL: Req RATE: 16000, real rate: 15999.987, BITS: 24, ERROR: 0.000081%
I (646) I2S: APLL: Req RATE: 16000, real rate: 16000.003, BITS: 32, ERROR: 0.000018%

I (662) I2S: APLL: Req RATE: 22050, real rate: 22049.982, BITS: 16, ERROR: 0.000080%
I (677) I2S: APLL: Req RATE: 22050, real rate: 22049.984, BITS: 24, ERROR: 0.000071%
I (693) I2S: APLL: Req RATE: 22050, real rate: 22049.994, BITS: 32, ERROR: 0.000027%

I (708) I2S: APLL: Req RATE: 32000, real rate: 32000.006, BITS: 16, ERROR: 0.000018%
I (724) I2S: APLL: Req RATE: 32000, real rate: 31999.974, BITS: 24, ERROR: 0.000081%
I (739) I2S: APLL: Req RATE: 32000, real rate: 32000.006, BITS: 32, ERROR: 0.000018%

I (755) I2S: APLL: Req RATE: 44100, real rate: 44099.988, BITS: 16, ERROR: 0.000027%
I (770) I2S: APLL: Req RATE: 44100, real rate: 44099.969, BITS: 24, ERROR: 0.000071%
I (786) I2S: APLL: Req RATE: 44100, real rate: 44099.988, BITS: 32, ERROR: 0.000027%

I (801) I2S: APLL: Req RATE: 48000, real rate: 47999.961, BITS: 16, ERROR: 0.000081%
I (816) I2S: APLL: Req RATE: 48000, real rate: 48000.016, BITS: 24, ERROR: 0.000033%
I (832) I2S: APLL: Req RATE: 48000, real rate: 47999.992, BITS: 32, ERROR: 0.000016%

I (847) I2S: APLL: Req RATE: 96000, real rate: 95999.984, BITS: 16, ERROR: 0.000016%
I (863) I2S: APLL: Req RATE: 96000, real rate: 96000.031, BITS: 24, ERROR: 0.000033%
I (878) I2S: APLL: Req RATE: 96000, real rate: 95999.984, BITS: 32, ERROR: 0.000016%

The unmodified version:

I (516) I2S: APLL: Req RATE: 10675, real rate: 10675.000, BITS: 16, ERROR: 0.000000%
I (527) I2S: APLL: Req RATE: 10675, real rate: 10675.003, BITS: 24, ERROR: 0.000030%
I (543) I2S: APLL: Req RATE: 10675, real rate: 10675.006, BITS: 32, ERROR: 0.000055%

I (558) I2S: APLL: Req RATE: 11025, real rate: 11024.991, BITS: 16, ERROR: 0.000080%
I (573) I2S: APLL: Req RATE: 11025, real rate: 11024.992, BITS: 24, ERROR: 0.000071%
I (589) I2S: APLL: Req RATE: 11025, real rate: 11024.991, BITS: 32, ERROR: 0.000080%

I (604) I2S: APLL: Req RATE: 16000, real rate: 15999.986, BITS: 16, ERROR: 0.000085%
I (620) I2S: APLL: Req RATE: 16000, real rate: 15999.987, BITS: 24, ERROR: 0.000081%
I (635) I2S: APLL: Req RATE: 16000, real rate: 16000.003, BITS: 32, ERROR: 0.000018%

I (651) I2S: APLL: Req RATE: 22050, real rate: 22049.982, BITS: 16, ERROR: 0.000080%
I (666) I2S: APLL: Req RATE: 22050, real rate: 22049.984, BITS: 24, ERROR: 0.000071%
I (682) I2S: APLL: Req RATE: 22050, real rate: 21972.619, BITS: 32, ERROR: 0.350934%

I (697) I2S: APLL: Req RATE: 32000, real rate: 32000.006, BITS: 16, ERROR: 0.000018%
I (712) I2S: APLL: Req RATE: 32000, real rate: 31999.974, BITS: 24, ERROR: 0.000081%
I (728) I2S: APLL: Req RATE: 32000, real rate: 32000.006, BITS: 32, ERROR: 0.000018%

I (743) I2S: APLL: Req RATE: 44100, real rate: 43945.238, BITS: 16, ERROR: 0.350934%
I (759) I2S: APLL: Req RATE: 44100, real rate: 44099.969, BITS: 24, ERROR: 0.000071%
I (774) I2S: APLL: Req RATE: 44100, real rate: 43945.238, BITS: 32, ERROR: 0.350934%

I (790) I2S: APLL: Req RATE: 48000, real rate: 47999.961, BITS: 16, ERROR: 0.000081%
I (805) I2S: APLL: Req RATE: 48000, real rate: 46874.922, BITS: 24, ERROR: 2.343913%
I (821) I2S: APLL: Req RATE: 48000, real rate: 43945.238, BITS: 32, ERROR: 8.447420%

I (836) I2S: APLL: Req RATE: 96000, real rate: 87890.477, BITS: 16, ERROR: 8.447420%
I (852) I2S: APLL: Req RATE: 96000, real rate: 96000.031, BITS: 24, ERROR: 0.000033%
I (867) I2S: APLL: Req RATE: 96000, real rate: 107421.875, BITS: 32, ERROR: 11.897786%

@mbt28
Copy link

mbt28 commented Mar 30, 2020

Try the attached patch. It has solved the inaccurate apll clock rate issue. The patch is w.r.t. latest IDF i.e. 4.0. Kindly help to share the result.
0001-driver-i2s-fix-apll_clock_rate-for-different-sample-.zip

The attached patch give is percentage difference of 0.001.

Hi,

How can I apply this patch file on Windows?

Thanks

jokubasver added a commit to jokubasver/saab-93NG-bluetooth-aux that referenced this issue Dec 19, 2021
This seems to have been fixed ages ago in ESP-IDF:
espressif/esp-idf#3648

espressif/esp-idf#3380
@espressif-bot espressif-bot added Resolution: Done Issue is done internally Status: Done Issue is done internally labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

8 participants