Skip to content
This repository was archived by the owner on Jan 14, 2022. It is now read-only.

Sample rate not ok? #3

Closed
Xyp opened this issue Oct 25, 2015 · 10 comments
Closed

Sample rate not ok? #3

Xyp opened this issue Oct 25, 2015 · 10 comments

Comments

@Xyp
Copy link

Xyp commented Oct 25, 2015

The function i2sSetRate in i2s_freertos.c did not work properly. When measuring the i2s wordselect frequency with then one, the function actually calculated (variable bestfreq), there was quite a deviation.
The accoustic effect was a change of sound-pitch.

The divider "calculation" in the below fix is not the most elegant one, but at least I think I identified the real formula for the word select formula (see code, line that begins with tstfreq=). The Base frequency is also 160 MHz (Independent on CPU speed setting).

Cannot commit the fix myself. So here the solution, which works for me.
Have also a second order sigma delta converter with over sampling ration of 64 working, delivering better sound quality (effectively 13.3 bit resolution, compared to 10 bits). E.g. speach does not sound that metallic anymore as before. Of course the noise on the IO pin itself will decrease the ENOB with respect to the ideal one.

So... here the sample rate fix code:

#define ABS(x) (((x)>0)?(x):(-(x)))

//Set the I2S sample rate, in HZ
void i2sSetRate(int rate) {
    //Find closest divider 
    int bestbck=0, bestfreq=0;
    int tstfreq;
    int i;
    int div;

    div = 2; // 2 is the minimal supported divider (divider 1 ends up in divider 2 setting)
    if (rate < 40000) // 39682 is the minimal sampling frequency supported with divider 2
        div = 3; // this one supports minimal 19841Hz

    //The base divider can be off by as much as <1 Compensate by trying to make the amount of bytes in the
    //i2s cycle more than 16. Do this by trying the amounts from 16 to 32 and keeping the one that fits best.
    for (i=1; i<64; i++) {
        tstfreq=(160000000l / (i*div*32) );
        printf("Best (%d,%d) cur (%d,%d) div %d\n", bestbck, bestfreq, i, tstfreq, ABS(rate-tstfreq));
        if (ABS(rate-tstfreq)<ABS(rate-bestfreq)) {
            bestbck=i;
            bestfreq=tstfreq;
        }
    }

    printf("ReqRate %d Div %d Bck %d Frq %d\n", rate, div, bestbck, bestfreq);
    CLEAR_PERI_REG_MASK(I2SCONF, I2S_TRANS_SLAVE_MOD|
                        (I2S_BITS_MOD<<I2S_BITS_MOD_S)|
                        (I2S_BCK_DIV_NUM <<I2S_BCK_DIV_NUM_S)|
                        (I2S_CLKM_DIV_NUM<<I2S_CLKM_DIV_NUM_S));
    SET_PERI_REG_MASK(I2SCONF, I2S_RIGHT_FIRST|I2S_MSB_RIGHT|I2S_RECE_SLAVE_MOD|
                        I2S_RECE_MSB_SHIFT|I2S_TRANS_MSB_SHIFT|
                        (((bestbck)&I2S_BCK_DIV_NUM )<<I2S_BCK_DIV_NUM_S)|
                        (((div)&I2S_CLKM_DIV_NUM)<<I2S_CLKM_DIV_NUM_S));
}
@Xyp
Copy link
Author

Xyp commented Oct 26, 2015

Theory why the formula can be correct:
Esp8266 seems to have a 160Mhz PLL. The I2s IP block can get a divided down version of this clock (variable div). The bitclock is then divided down further by the integer bestbck value. The WS signal (=samplerate) is then the BCK divided by 32.
I2s has no fractional divider/own pll, though, so most standard samplerates can not be met 100%.

Would be so cool if the IP blocks would be publicly documented, rather than having only an API available. An API does not fully respect the capabilities.
Would love to have also i2s input working, but this is hard to do without knowing also how DMA can be setup for I2S input.

@DagAgren
Copy link

Can confirm that the code in that section and the comments are completely wrong.

There does seem to be a 160 MHz clock source, that gets divided twice by the BCK_DIV and CLKM_DIV values. And there should be no -1 when setting the register, either. The values are used directly by the hardware. (No idea what happens if you try to divide by zero!)

The original code works only by pure chance. It seems the correct way to set the rate is indeed to find two divisors that divide 160 MHz down to 32 times your preferred sample rate.

@DagAgren
Copy link

Any proper code would have to try and calculate (160000000+samplerate16)/(samplerate32), and then try to find two numbers, 1..63 and 2..63, whose product is as close as possible to this number. For instance, 8*13=104 works fairly well to get 48 kHz.

Alternatively, make a table of dividers for common sample rates, and only allow those.

@Xyp
Copy link
Author

Xyp commented Nov 15, 2015

For a more precies samplerate it would be possible to use an I2S codec or I2S DAC, that acts as i2s master. This would require an i2c communication to setup the codec properly each time i2sSetRate() is called.

As there is no asynchronous sample rate conversion done in the current setup ("only" skipping/doubling of samples) this would give better quality audio, as skipping/doubling occurs less often.

@Spritetm
Copy link
Member

According to our engineers:
CLK_I2S = 160MHz / I2S_CLKM_DIV_NUM
BCLK = CLK_I2S / I2S_BCK_DIV_NUM
WS = BCLK/ 2 / (16 + I2S_BITS_MOD)
Note that I2S_CLKM_DIV_NUM must be >5 for I2S data

It will be added into the documentation, and I'll change the code accordingly.

@Spritetm
Copy link
Member

Fixed in 75cd867

@DagAgren
Copy link

What actually happens if you set I2S_BITS_MOD to something other than 0? How does that look on the actual bus?

@Spritetm
Copy link
Member

From what I remember, the hardware will output extra low bits on the bus after the 16 bits for each channel have been sent out. Don't have a scope here so I can't check.

@DagAgren
Copy link

In that case, your PWM and sigma-delta code will not work as expected with this update.

@Spritetm
Copy link
Member

I would say it will. The delta-sigma samples will be pushed together a bit and the average volume will be slightly lower, but because the changes will be above the sample rate anyway it shouldn't impact any signal below the nyquist frequency, apart from the decrease in volume.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants