Update I2S base frequency and frequency dividers to real ones

[Crypter]

Values taken from:
https://github.com/espressif/ESP8266_MP3_DECODER/blob/master/mp3/driver/i2s_freertos.c

Reference:
espressif/ESP8266_MP3_DECODER@75cd867

[Crypter]

Relevant discussion here:
espressif/ESP8266_MP3_DECODER#3

Also please merge the new I2S features and fixes provided by other users, this part of the SDK is lacking a lot of stuff, and I'm waiting on it so that I can finish a library I'm writing

[devyte]

@Crypter in general, proposed I2S features will be left for 2.5.0 release, meaning the intent is to merge post 2.4.0 release. This does not apply to bug fixes.
I believe there is currently no active developer in charge of I2S, so I need verification from other users for a specific PR before merging. Hence:
-Which PRs have you tested?
-Are they bugs of enhancements?
-What feedback can you provide?

[earlephilhower]

Thanks for the PR!

It's an interesting start, but it looks to me like more changed needed to pull in the work from the NONOS-MP3 player.

[earlephilhower]

What if the CPU is not in 2x mode?

[Crypter]

@earlephilhower I'm glad you asked all of this.

Cnlohr confirmed small part of this in one of his projects/videos, there was confirmation in one of the links above, and I just confirmed it experimentaly in my room using a microphone and a speaker attached to the I2S output pins.

Apparently there's internal PLL running on 160MHz, completely independent from the CPU clock. That is divided by two deviders, the first one is the so-called clock divider, and the later is bck (word select) divider. All of that naming doesn't make sense, because the first divider just divides the clock to a lower, closer to what we need, number, and the second one actually gives us the clock speed of the I2S bus. ( lines 215, 216 in the file, uint32_t i2s_clock_div = THIS_NUMBER & I2SCDM; uint8_t i2s_bck_div = AND_THIS_NUMBER & I2SBDM; )

For example, on 160MHz, if you set the first divider to 19, and the second one to 6 you'll get ~1403508.7 Hz I2S clock speed, from which later is derived the Word select clock by dividing that to 32(bits), which gives us ~43859.64 Hz Word select clock. This is the closest you can get to 44.1KHz.

When I set the dividers to their maximum of 63 (64 overflows), I got ~40312 Hz clock speed, which when I attached a speaker to it and recorded that on 96KHz sample rate (max of my soundcard), I found the peak very clearly on my audio spectral analyzer. And the corresponding WS clock at 1259-1260Hz too, which is very audible also.

I encourage you (reader) to test this on oscilloscope, because I don't own one, but I'm 100% sure in my findings and calculations.

Which follows that the audio coming out of this chip using I2S would be somewhat slower and with lower pitch, but that's a completely different topic.

[Crypter]

Oh, and we'll have to find a more decent way to set the sample rate more accurately because this doesn't work at all. I suggest a precomputed look up table with the most common sample rates.

[Crypter]

I precomputed the following values:

This is the closest we can get from the real numbers. Even a simple switch-case will do the job here, I'll implement it later today. Maybe we should provide a new function that will enable us to provide the dividers directly for better control?

[earlephilhower]

Cool, thanks a lot for the info and add'l update! I'll give the existing change a pull and run it against my audio stack this week. I need to pore over the existing code because it still seems that such a large constant change would have a correspondingly large frequency change, but it'll be simple enough to test once I get some headphones! I'll also see if my DMM has a freq counter option.

I would caution against a hard-coded table because when the I2S is used (abused) as a software sigma-delta DAC you can do 64x and 96x oversampling to get more effective bits. My own AAC/MP3/etc. audio stack does this now. You end up requesting an output frequency of (baserate*oversample/32) so you end up with requests for, say, 88.2Khz for a 44.1 @ 64x oversamples.

So you're thinking of the existing setFreq(x hz) and an optional setDividers(div1, div2) call? If setFreq does its job, there's not much more that setDividers can do, no?

Thx!
-EFP3

[earlephilhower]

Thank you VERY much! for such a detailed analysis!

Would you be able to do the brute force as a change? I thought that was already being done in the MP3 example, so it might simply be a cut-n-paste. I'd stick with lockbits=1 so only try 16 bits, because the SW DAC doesn't actually generate those other bits and you'll introduce a bias in the output signal by putting in 0s, so it comes to (64-2)*(64-5) iterations:

	for (bckdiv=2; bckdiv<64; bckdiv++) {
		for (clkmdiv=5; clkmdiv<64; clkmdiv++) {
			tstfreq=BASEFREQ/(bckdiv*clkmdiv*16*2);
			if (ABS(rate-tstfreq)<ABS(rate-bestfreq)) {
				bestfreq=tstfreq;
				bestclkmdiv=clkmdiv;
				bestbckdiv=bckdiv;
			}
		}
	}
```

[earlephilhower]

And if you can add a getActualFreq call, a user application can actually correct for this difference between requested and generated via simple dropping our replicating samples, no?

[Crypter]

In theory - yes. In practice it takes some interpolation and dithering to achieve better results. This can be a problem only during playback of streams for longer periods due to buffer overrun or underruns by different feed and playback rates. In practice the minor network connection issues (jitter and packet loss) will solve this issue by forcing re-buffering of the stream once in a while. We replace our issue with a better issue. Networking rocks!

Also, once upon a time 4% difference in speed was acceptable... :)

[Crypter]

Of course, all of the above applies only and only if the ESP8266 playing the stream and the server serving the stream have zero relative drift in their clocks. Funny how imperfect technology is...

[earlephilhower]

Thanks for such a fast turnaround!

I'm in the middle of rocket surgery tonight, actually, working with @tueddy porting the ESP8266Audio library to the ESP32 (just plugged in after about 3 months sitting under my keyboard), but I promise to look at this by the weekend and get it into the merge window for 2.4.1 or 2.5.0 (whichever comes next)!

[earlephilhower]

Looking at the math in this function vs. what's in the MP3_decoder vs. what's there now, I can't see how increasing I2SBASEFREQ by 13.33x and then incrementing the divider (aka div-by-2) would work. Assuming it works at all, wouldn't this just increase the output frequency by 13.33/2 or 13.33/4?

The MP3 decoder reference has a series of configs on the clock source for I2S and then a large for-loop to determine the closest divider with their (different, I believe) clock inputs.
https://github.com/espressif/ESP8266_MP3_DECODER/blob/7552a62d425598d64ebc255d32fa4f20220e92f6/mp3/driver/i2s_freertos.c#L250

Have you run this with a frequency counter on LRCLK for different CPU freqs and sample rates? The existing logic is close enough for most sample outputs I've seen for multi-hour AAC and MP3 streams, so seeing such a large change in clocking requested here really puzzles me.

[Crypter]

I2S fix applied, can now be tested.

[tueddy]

Hi earlephilhower,
thank's for supporting ESP32! I see there is a lot of work to do, good way to add a build/test system.
I'll go with this platform with my kidsplayer project and try to help as i can!
Best Dirk

[earlephilhower]

@Crypter Thanks again!