8000 I2S switches from MSB-first to LSB-first on an ESP32-S3 · Issue #11245 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

I2S switches from MSB-first to LSB-first on an ESP32-S3 #11245

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

Jump to bottom
Closed
ma261065 opened this issue Apr 12, 2023 · 8 comments
Closed

I2S switches from MSB-first to LSB-first on an ESP32-S3 #11245

ma261065 opened this issue Apr 12, 2023 · 8 comments
Labels
bug

Comments

@ma261065
Copy link
ma261065 commented Apr 12, 2023 •
edited
Loading

Using the latest nightly build (v1.19.1-1014-gbde222ce8 (2023-04-11) ) on an ESP32-S3 with SPIRAM, I am sending a wav file that is stored on the onboard filesystem to a PCM5102A DAC via I2S.

When I run the code the first time after power on or reset (soft or hard) the audio is heard clearly.

If I then run the same code again, it is just a mess of static - I can just hear the beat of the music in all the noise though.

The only way to get it working again is to reboot. This happens on I2S0 as well as I2S1, and have tried different pins in case I was clashing with one of the ESP32 pins that has alternative functions.

Also, when rebooting I see the following message:
E (130390) I2S: i2s_driver_uninstall(2047): I2S port 0 has not installed
just before the MPY: soft reboot message

I am not getting this problem on a non-S3 variant.

Code below:

from machine import I2S, Pin
import io

sck_pin = Pin(13)   # Serial clock output
ws_pin = Pin(14)    # Word clock output
sd_pin = Pin(17)    # Serial data output

buf = bytearray(1024)

audio_out = I2S(0, sck=sck_pin, ws=ws_pin, sd=sd_pin, mode=I2S.TX, bits=16, format=I2S.STEREO, rate=16000, ibuf=20000)

with io.open("example.wav", 'rb') as fp:
    size = fp.readinto(buf)

    while (size > 0):
        print('.', end='')
        audio_out.write(buf)
        size = fp.readinto(buf)
    
print("Finished")    
fp.close()
audio_out.deinit()
@ma261065 ma261065 added the bug label Apr 12, 2023
@ma261065
Copy link
Author
ma261065 commented Apr 20, 2023 •
edited
Loading

I have had a look at the output using a logic analyser, and when it goes all noisy the cause is the data being sent LSB first instead of MSB fir 8000 st.

My example.wav file has the first four bytes as 0x52 0x49 0x46 0x46

When it's working I see this on the logic analyser:
5249 4646
01010010 01001001 01000110 01000110

When it goes all noisy, I see this:
4A92 6262
01001010 10010010 01100010 01100010

WhatsApp Image 2023-04-21 at 14 02 57

WhatsApp Image 2023-04-21 at 14 13 15

(I know the data shown above is the first part of the wav header, but to simplify the repro code I have chosen not to skip that. I know it will cause a little audio glitch at the beginning of the playback while it "plays" the header, but that is irrelevant to this issue. When it's working I hear [glitch][clear audio file], when it's not working I hear [glitch][static])

@ma261065
Copy link
Author
ma261065 commented Apr 22, 2023 •
edited
Loading

I don't know if this is relevant or not, but from the ESP-IDF docs, I note that the i2s_std_slot_config_t struct for the ESP32-S3 is different to a normal ESP32, in that the ESP32-S3 one contains an extra two bools at the end, and changes the meaning of the last bool:

ESP32
...
...
bool bit_shift - Set to enable bit shift in Philips mode
bool msb_right - Set to place right channel data at the MSB in the FIFO

vs

ESP32-S3
...
...
bool bit_shift - Set to enable bit shift in Philips mode
bool left_align - Set to enable left alignment
bool big_endian - Set to enable big endian
bool bit_order_lsb - Set to enable lsb first

So I guess if we are passing the ESP32 i2s_std_slot_config_t struct to an ESP32-S3, it will read a bigger amount of memory than we are expecting as it is expecting a bigger struct. Therefore the last two bools will be off in random memory somewhere and could lead to inconsistent behaviour. And given that one of them controls MSB-first/LSB-first behaviour, this could explain the issue. Am I right?

EDIT: Ahh, on further reading, this struct seems to be used by the "new API" only, which I don't think MicroPython uses, so is probably not relevant. So, now I'm out of ideas...

Help - anyone?

@ma261065 ma261065 changed the title I2S only works right after reset on ESP32-S3 I2S switches from MSB-first to LSB-first on an ESP32-S3 Apr 22, 2023
@ma261065
Copy link
Author
ma261065 commented May 3, 2023 •
edited
Loading

OK - I think I have it figured out.

In esp-idf/components/driver/deprecated/driver/i2s_types_legacy.h we have the following IF-delimited extra section in the i2s_config_t struct, if the chip supports TDM:

...
...
#if SOC_I2S_SUPPORTS_TDM
    i2s_channel_t           chan_mask;                  /*!< I2S active channel bit mask, set value in i2s_channel_t to enable specific channel, the bit map of active channel can not exceed (0x1<<total_chan). */
    uint32_t                total_chan;                 /*!< I2S Total number of channels. If it is smaller than the biggest active channel number, it will be set to this number automatically. */
    bool                    left_align;                 /*!< Set to enable left alignment */
    bool                    big_edin;                   /*!< Set to enable big endian */
    bool                    bit_order_msb;              /*!< Set to enable msb order */
    bool                    skip_msk;                   /*!< Set to enable skip mask. If it is enabled, only the data of the enabled channels will be sent, otherwise all data stored in DMA TX buffer will be sent */
#endif // SOC_I2S_SUPPORTS_TDM

Currently the ESP32C3, ESP32C6 & ESP32S3 support TDM

I had always thought that uninitialised members of a struct in C got set to default values, but it seems that this isn't happening here**.

In machine_i2s.c if I add the following to the end where machine_i2s_init_helper populates the i2s_config struct, then it all works reliably, and doesn't change behaviour between executions.

#if SOC_I2S_SUPPORTS_TDM
    i2s_config.chan_mask = 0x03;
    i2s_config.total_chan = 0;
    i2s_config.left_align = true;
    i2s_config.big_edin = false;
    i2s_config.bit_order_msb = false;
    i2s_config.skip_msk = false;
#endif

** I can't explain this, but if I put debug printf statements in machine_i2s_init_helper, those bools always show as true even though machine_i2s_init_helper doesn't set them, validating my thinking that they are being set to default values. However the behaviour still varies between executions, even though the printf always shows the same. If I explicitly set them as per the above, the random behaviour disappears.

@ma261065 ma261065 mentioned this issue May 29, 2023
Open
@ma261065 ma261065 mentioned this issue Jul 22, 2023
Open
ricksorensen added a commit to ricksorensen/micropython that referenced this issue Jul 22, 2023
@ricksorensen
Set i2s_config parameters directly
f97d93c 
This helps avoid the static play on the C3 (S3?).  Changes are from
micropython#11245
thanks to Mike Alexander
@ricksorensen ricksorensen mentioned this issue Jul 25, 2023
Open
@MATTYGILO
Copy link

Hi I am having the same problem, has this been merged into micropython yet?

MATTYGILO added a commit to MATTYGILO/micropython that referenced this issue Nov 4, 2023
@MATTYGILO
SOC_I2S_SUPPORTS_TDM
f8be498 
micropython#11245
@MATTYGILO
Copy link

OK - I think I have it figured out.

In esp-idf/components/driver/deprecated/driver/i2s_types_legacy.h we have the following IF-delimited extra section in the i2s_config_t struct, if the chip supports TDM:

...
...
#if SOC_I2S_SUPPORTS_TDM
    i2s_channel_t           chan_mask;                  /*!< I2S active channel bit mask, set value in i2s_channel_t to enable specific channel, the bit map of active channel can not exceed (0x1<<total_chan). */
    uint32_t                total_chan;                 /*!< I2S Total number of channels. If it is smaller than the biggest active channel number, it will be set to this number automatically. */
    bool                    left_align;                 /*!< Set to enable left alignment */
    bool                    big_edin;                   /*!< Set to enable big endian */
    bool                    bit_order_msb;              /*!< Set to enable msb order */
    bool                    skip_msk;                   /*!< Set to enable skip mask. If it is enabled, only the data of the enabled channels will be sent, otherwise all data stored in DMA TX buffer will be sent */
#endif // SOC_I2S_SUPPORTS_TDM

Currently the ESP32C3, ESP32C6 & ESP32S3 support TDM

I had always thought that uninitialised members of a struct in C got set to default values, but it seems that this isn't happening here**.

In machine_i2s.c if I add the following to the end where machine_i2s_init_helper populates the i2s_config struct, then it all works reliably, and doesn't change behaviour between executions.

#if SOC_I2S_SUPPORTS_TDM
    i2s_config.chan_mask = 0x03;
    i2s_config.total_chan = 0;
    i2s_config.left_align = true;
    i2s_config.big_edin = false;
    i2s_config.bit_order_msb = false;
    i2s_config.skip_msk = false;
#endif

** I can't explain this, but if I put debug printf statements in machine_i2s_init_helper, those bools always show as true even though machine_i2s_init_helper doesn't set them, validating my thinking that they are being set to default values. However the behaviour still varies between executions, even though the printf always shows the same. If I explicitly set them as per the above, the random behaviour disappears.

Can confirm this works

@dpgeorge
Copy link
Member
dpgeorge commented Mar 8, 2024

This may be fixed by 0b145fd

@ma261065
Copy link
Author
ma261065 commented Apr 8, 2024 •
edited
Loading

This may be fixed by 0b145fd

Confirmed that this update to the new I2S driver fixes the issue without requiring the workaround.

@ma261065
Copy link
Author
ma261065 commented Apr 8, 2024

Fixed by 0b145fd

@ma261065 ma261065 closed this as completed Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dpgeorge @ma261065 @MATTYGILO
0