Skip to content

Update I2S base frequency and frequency dividers to real ones #4031

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jan 20, 2018
Merged

Update I2S base frequency and frequency dividers to real ones #4031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
33a3112
Fix I2S base frequency
Crypter Dec 27, 2017
d1496a4
Merge pull request #1 from Crypter/hotfix/i2s_frequency
Crypter Dec 27, 2017
f335b28
Update frequency dividers for I2S
Crypter Dec 27, 2017
b07cc34
Merge branch 'master' into master
earlephilhower Jan 15, 2018
a380742
Merge branch 'master' into master
Crypter Jan 17, 2018
941e68e
fixed i2s_set_rate, added i2s_get_real_rate() and i2s_set_dividers
Jan 17, 2018
b56bc49
Minimise float calculations
Crypter Jan 18, 2018
6df60c4
Merge branch 'master' into master
Crypter Jan 18, 2018
f5d676a
Merge branch 'master' into master
earlephilhower Jan 20, 2018
f67cff8
Merge branch 'master' into master
earlephilhower Jan 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions cores/esp8266/core_esp8266_i2s.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,43 @@ static uint32_t _i2s_sample_rate;
void ICACHE_FLASH_ATTR i2s_set_rate(uint32_t rate){ //Rate in HZ
if(rate == _i2s_sample_rate) return;
_i2s_sample_rate = rate;
uint32_t i2s_clock_div = (I2SBASEFREQ/(_i2s_sample_rate*32)) & I2SCDM;
uint8_t i2s_bck_div = (I2SBASEFREQ/(_i2s_sample_rate*i2s_clock_div*2)) & I2SBDM;

uint32_t scaled_base_freq = I2SBASEFREQ/32;
float delta_best = scaled_base_freq;

uint8_t sbd_div_best=1;
uint8_t scd_div_best=1;
for (uint8_t i=1; i<64; i++){
for (uint8_t j=i; j<64; j++){
float new_delta = fabs(((float)scaled_base_freq/i/j) - rate);
if (new_delta < delta_best){
delta_best = new_delta;
sbd_div_best = i;
scd_div_best = j;
}
}
}

//os_printf("Rate %u Div %u Bck %u Frq %u\n", _i2s_sample_rate, i2s_clock_div, i2s_bck_div, I2SBASEFREQ/(i2s_clock_div*i2s_bck_div*2));

//!trans master, !bits mod, rece slave mod, rece msb shift, right first, msb right
I2SC &= ~(I2STSM | (I2SBMM << I2SBM) | (I2SBDM << I2SBD) | (I2SCDM << I2SCD));
I2SC |= I2SRF | I2SMR | I2SRSM | I2SRMS | ((i2s_bck_div-1) << I2SBD) | ((i2s_clock_div-1) << I2SCD);
I2SC |= I2SRF | I2SMR | I2SRSM | I2SRMS | ((sbd_div_best) << I2SBD) | ((scd_div_best) << I2SCD);
}

void ICACHE_FLASH_ATTR i2s_set_dividers(uint8_t div1, uint8_t div2){
div1 &= I2SBDM;
div2 &= I2SCDM;

I2SC &= ~(I2STSM | (I2SBMM << I2SBM) | (I2SBDM << I2SBD) | (I2SCDM << I2SCD));
I2SC |= I2SRF | I2SMR | I2SRSM | I2SRMS | (div1 << I2SBD) | (div2 << I2SCD);
}

float ICACHE_FLASH_ATTR i2s_get_real_rate(){
return (float)I2SBASEFREQ/32/((I2SC>>I2SBD) & I2SBDM)/((I2SC >> I2SCD) & I2SCDM);
}


void ICACHE_FLASH_ATTR i2s_begin(){
_i2s_sample_rate = 0;
i2s_slc_begin();
Expand Down
2 changes: 1 addition & 1 deletion cores/esp8266/esp8266_peri.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ extern uint8_t esp8266_gpioToFn[16];
#define i2c_bbpll_en_audio_clock_out_msb 7
#define i2c_bbpll_en_audio_clock_out_lsb 7
#define I2S_CLK_ENABLE() i2c_writeReg_Mask_def(i2c_bbpll, i2c_bbpll_en_audio_clock_out, 1)
#define I2SBASEFREQ (12000000L)
#define I2SBASEFREQ (160000000L)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the CPU is not in 2x mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I precomputed the following values:
image

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
			}
		}
	}
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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... :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


#define I2STXF ESP8266_REG(0xe00) //I2STXFIFO (32bit)
#define I2SRXF ESP8266_REG(0xe04) //I2SRXFIFO (32bit)
Expand Down
4 changes: 3 additions & 1 deletion cores/esp8266/i2s.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ extern "C" {
void i2s_begin();
void i2s_end();
void i2s_set_rate(uint32_t rate);//Sample Rate in Hz (ex 44100, 48000)
void i2s_set_dividers(uint8_t div1, uint8_t div2);//Direct control over output rate
float i2s_get_real_rate();//The actual Sample Rate on output
bool i2s_write_sample(uint32_t sample);//32bit sample with channels being upper and lower 16 bits (blocking when DMA is full)
bool i2s_write_sample_nb(uint32_t sample);//same as above but does not block when DMA is full and returns false instead
bool i2s_write_lr(int16_t left, int16_t right);//combines both channels and calls i2s_write_sample with the result
Expand All @@ -54,4 +56,4 @@ int16_t i2s_available();// returns the number of samples than can be written bef
}
#endif

#endif
#endif