Audio pops and dropouts when using I2C

njswede

Member
In my application, I occasionally send realtime data to a display over I2C. This sometimes results in pops and dropouts, which are especially noticeable at high input levels. I'm using the Audio Library in slave mode. Not sure that's related.

Here's the relevant code:

#include <Arduino.h> #include <Audio.h> #include <Wire.h> AudioInputI2Sslave i2sInput; // Audio input from I2S AudioOutputI2Sslave i2sOutput; // Audio output to I2S AudioAnalyzeRMS rms; // RMS analyzer AudioConnection patchCord1(i2sInput, 0, rms, 0); // Connect input to RMS analyzer AudioConnection myPatchCord1(i2sInput, 0, i2sOutput, 0); // Connect input to output left channel AudioConnection myPatchCord2(i2sInput, 1, i2sOutput, 1); // Connect input to output right channel void setup() { // put your setup code here, to run once: AudioMemory(12); Wire.begin(); // Join I2C bus as master Serial.begin(115000); delay(1000); // Wait for serial to initialize Serial.println("Starting I2S Slave Audio Example"); //i2sInput.begin(); //i2sOutput.begin(); } #define FILTER_BANDS 5 typedef struct { uint8_t packetType; uint8_t selectedFilterBand; uint8_t displayMode; uint8_t reserved[1]; // Padding for alignment union { struct { uint8_t reserved[3]; // Padding for alignment uint32_t b0; // Encoded as network order float. Use htonf() to convert. uint32_t b1; uint32_t b2; uint32_t a1; uint32_t a2; } coeffs[FILTER_BANDS]; struct { uint8_t filterType; uint8_t reserved[3]; // Padding for alignment uint32_t frequency; uint32_t Q; uint32_t gain; } params[FILTER_BANDS]; struct { uint32_t peakLevel; uint32_t rmsLevel; } status; struct { uint32_t sampleRate; uint8_t stable; uint8_t bits; uint8_t reserved[2]; // Padding for alignment } sampleRateInfo; struct { uint8_t clipped; uint8_t reserved[3]; // Padding for alignment } clipAlert; } data; uint32_t checksum; // Network order checksum of all previous bytes in the packet } Packet; #define PACKET_CLIP_ALERT 0x05 uint32_t htonl(uint32_t hostlong) { #if ( __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ ) return __builtin_bswap32(hostlong); #elif ( __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ ) return hostlong; #else #error "integer byte order could not be determined!" #endif } uint32_t crc32Buffer(const uint8_t *data, size_t length) { uint32_t crc = 0xFFFFFFFF; for (size_t i = 0; i < length; i++) { crc ^= data[i]; for (uint8_t j = 0; j < 8; j++) { if (crc & 1) { crc = (crc >> 1) ^ 0xEDB88320; } else { crc >>= 1; } } } return ~crc; } void sendToDisplay(Packet *packet) { packet->checksum = htonl(crc32Buffer((uint8_t *)packet, sizeof(Packet) - 4)); Wire.beginTransmission(0xb1ce); Wire.send((uint8_t *)packet, sizeof(Packet)); Wire.endTransmission(); } void loop() { Packet p; p.packetType = PACKET_CLIP_ALERT; p.data.clipAlert.clipped = 1; sendToDisplay(&p); delay(100); p.data.clipAlert.clipped = 0; sendToDisplay(&p); delay(100); Serial.println("Hello"); }
 
Can't see anything very odd there, but I do have a few comments.

First, when posting a block of code you're better off using </> button, not the inline >_ one.

Second, what are you expecting when you have Wire.beginTransmission(0xb1ce); in your code? I²C addresses are 7 bits, not 16.

Thirdly, this looks strange:
C++:
        struct {
            uint8_t reserved[3]; // Padding for alignment <<< why?!
            uint32_t b0; // Encoded as network order float. Use htonf() to convert.
            uint32_t b1;
            uint32_t b2;
            uint32_t a1;
            uint32_t a2;
        } coeffs[FILTER_BANDS];
Maybe a copy / paste error?

I guess you have some sort of smart display of your own devising on the other end of the I²C bus. Those are quite big packets you're sending, and the FIFO is only 4 bytes long, so there's likely to be quite a lot of time spent waiting for transmission to complete (>2ms each time at 100kbits/s). The library does call yield() during the wait, I believe, though that ought not to cause any issues.
 
How would I use this code to reproduce the problem?

First, I tried copying into Arduino IDE. It doesn't compile. I get this error:

Code:
sketch_jan03a:83: error: variable or field 'sendToDisplay' declared void
   83 | void sendToDisplay(Packet *packet)
      |      ^~~~~~~~~~~~~
sketch_jan03a:83: error: 'Packet' was not declared in this scope
   83 | void sendToDisplay(Packet *packet)
      |                    ^~~~~~
sketch_jan03a:83: error: 'packet' was not declared in this scope
   83 | void sendToDisplay(Packet *packet)
      |                            ^~~~~~
 
The padding must have been some kind of copy/paste error, because it's not present in my original code.

Yes, I have one of those CYD (Cheap Yellow Display) units with a built-in ESP32, so I'm using I2C to send commands to the display. I could always split that packet up into smaller ones, like sending the coefficients for each filter in a separate packet.

If you haven't already guessed it, I'm building a parametric EQ. Mostly for fun.
 
Even if I can compile the code and run it here on a Teensy 4.0 or Teensy 4.1, what else do I need to connect?

Some sort of display is needed on the SDA and SCL pins, right? Is this an off-the-shelf item I can buy? Or is it something special, like another board running specific code?

Some sort of codec chip running in I2S master mode is needed on the I2S pins, right? Can I use this WM8731 audio board? Or do I need something else?

Once I have all the hardware connected, how do I observe the pops and dropouts? Since the RMS analysis seems to be unused, I'm guessing I need to observe the audio output somehow? Is this a subjective listening test?
 
How would I use this code to reproduce the problem?

First, I tried copying into Arduino IDE. It doesn't compile. I get this error:

Code:
sketch_jan03a:83: error: variable or field 'sendToDisplay' declared void
   83 | void sendToDisplay(Packet *packet)
      |      ^~~~~~~~~~~~~
sketch_jan03a:83: error: 'Packet' was not declared in this scope
   83 | void sendToDisplay(Packet *packet)
      |                    ^~~~~~
sketch_jan03a:83: error: 'packet' was not declared in this scope
   83 | void sendToDisplay(Packet *packet)
      |                            ^~~~~~

"Packet" should be declared right above that code.

It compiles fine in PlatformIO.

I think something got mangled when I pasted it. This compiles in Arduino IDE:

C++:
#include <Arduino.h>
#include <Audio.h>
#include <Wire.h>

AudioInputI2Sslave i2sInput;       // Audio input from I2S
AudioOutputI2Sslave i2sOutput;     // Audio output to I2S
AudioAnalyzeRMS          rms;           // RMS analyzer
AudioConnection          patchCord1(i2sInput, 0, rms, 0); // Connect input to RMS analyzer
AudioConnection myPatchCord1(i2sInput, 0, i2sOutput, 0); // Connect input to output left channel
AudioConnection myPatchCord2(i2sInput, 1, i2sOutput, 1); // Connect input to output right channel

#define FILTER_BANDS 5

typedef struct {
    uint8_t packetType;
    uint8_t selectedFilterBand;
    uint8_t displayMode;
    uint8_t reserved[1]; // Padding for alignment
    union {
        struct {
            uint32_t b0; // Encoded as network order float. Use htonf() to convert.
            uint32_t b1;
            uint32_t b2;
            uint32_t a1;
            uint32_t a2;
        } coeffs[FILTER_BANDS];
        struct {
            uint8_t filterType;
            uint8_t reserved[3]; // Padding for alignment
            uint32_t frequency;
            uint32_t Q;
            uint32_t gain;
        } params[FILTER_BANDS];
        struct {
            uint32_t peakLevel;
            uint32_t rmsLevel;
        } status;
        struct {
            uint32_t sampleRate;
            uint8_t stable;
            uint8_t bits;
            uint8_t reserved[2]; // Padding for alignment
        } sampleRateInfo;
        struct {
            uint8_t clipped;
            uint8_t reserved[3]; // Padding for alignment
        } clipAlert;
    } data;
    uint32_t checksum; // Network order checksum of all previous bytes in the packet
} Packet;

#define PACKET_CLIP_ALERT 0x05

uint32_t htonl(uint32_t hostlong) {
    #if ( __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ )
        return __builtin_bswap32(hostlong);
    #elif ( __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ )
        return hostlong;
    #else
        #error "integer byte order could not be determined!"
    #endif
}
uint32_t crc32Buffer(const uint8_t *data, size_t length) {
    uint32_t crc = 0xFFFFFFFF;
    for (size_t i = 0; i < length; i++) {
        crc ^= data[i];
        for (uint8_t j = 0; j < 8; j++) {
            if (crc & 1) {
                crc = (crc >> 1) ^ 0xEDB88320;
            } else {
                crc >>= 1;
            }
        }
    }
    return ~crc;
}

void sendToDisplay(Packet *packet)
{
  packet->checksum = htonl(crc32Buffer((uint8_t *)packet, sizeof(Packet) - 4));
  Wire.beginTransmission(0xb1ce);
  Wire.send((uint8_t *)packet, sizeof(Packet));
  Wire.endTransmission();
}


void setup() {
  // put your setup code here, to run once:
  AudioMemory(12);
  Wire.begin(); // Join I2C bus as master
  Serial.begin(115000);
  delay(1000); // Wait for serial to initialize
  Serial.println("Starting I2S Slave Audio Example");

  //i2sInput.begin();
  //i2sOutput.begin();
}



void loop() {
    Packet p;
    p.packetType = PACKET_CLIP_ALERT;
    p.data.clipAlert.clipped = 1;
    sendToDisplay(&p);
    delay(100);
    p.data.clipAlert.clipped = 0;
    sendToDisplay(&p);
    delay(100);
    Serial.println("Hello");
}
 
Confirm, I'm able to compile code from msg #6 in Arduino IDE.

Now 1 step closer, but still need quite a bit of info to be able to reproduce the problem here....

Just to be realistic, to have any hope of getting to the bottom of what's really going wrong, I need to reproduce the problem since nothing is apparently wrong (at least to my eyes) in the code.
 
Some sort of display is needed on the SDA and SCL pins, right? Is this an off-the-shelf item I can buy? Or is it something special, like another board running specific code?
Anything that can receive some I2C message. Preferably a rather long one. I think this message is 80 bytes. That might be too long. I can shorten it substantially by sending the coefficients for each filter separately.

It looks like the "main thread" code for the I2C library does very little. Basically just buffer management and the rest happens in ISRs. I'm not familiar with interrupt arbitration, but could it be that audio interrupts are blocked while the I2C ISR does its thing?
 
Now 1 step closer, but still need quite a bit of info to be able to reproduce the problem here....
That's OK. I'm not expecting a 10 minute service level on a Saturday evening. :)

You could probably hook up two Teensys that talk to each other. The code on the other side looks like this:

C++:
#ifndef COMMS_H_INCLUDED
#define COMMS_H_INCLUDED

#include "../../common/packets.h"

Packet *popPacket();
void processIncomingPacket(int numBytes);

#endif

C++:
#include "comms.h"

#include <Wire.h>
#include <Arduino.h>
#include <ErriezCRC32.h>
#include <netconv.h>
#include "../../common/packets.h"
#include "netconv.h"

#define PACKET_QUEUE_SIZE 10

Packet packetQueue[PACKET_QUEUE_SIZE];
volatile uint32_t queueHead = 0;
volatile uint32_t queueTail = 0;


/**
 * @brief Removes and returns the oldest packet from the packet queue.
 *
 * This function retrieves the packet at the head of the circular queue in a
 * thread-safe manner by disabling interrupts during the operation. After
 * retrieving the packet, it advances the head pointer and re-enables interrupts.
 *
 * @return Packet* Pointer to the oldest packet in the queue, or nullptr if the queue is empty.
 *
 * @note This function disables interrupts temporarily to ensure atomic access to the queue.
 * @note The returned pointer points to a buffer that may be reused when the queue wraps around.
 * @warning The caller should process or copy the packet data before it gets overwritten.
 */
Packet *popPacket()
{
    noInterrupts();
    if (queueHead == queueTail)
    {
        interrupts();
        return nullptr; // Queue empty
    }
    Packet *packet = &(packetQueue[queueHead]);
    queueHead = (queueHead + 1) % PACKET_QUEUE_SIZE;
    interrupts();
    return packet;
}

/**
 * @brief Processes an incoming I2C packet and adds it to the packet queue.
 *
 * This function must be called from an ISR context. It reads data from the I2C Wire
 * interface, validates the packet size and checksum, and adds it to a circular queue.
 *
 * @param numBytes The number of bytes available to read from the I2C buffer
 *
 * @note This function performs the following validations:
 *       - Checks for queue overflow before adding the packet
 *       - Verifies that the complete packet was received (exact size match)
 *       - Validates the CRC32 checksum of the received packet
 *
 * @warning Must be called from ISR context only
 * @warning If packet validation fails (incomplete, too large, or invalid checksum),
 *          the packet is discarded and not added to the queue
 *
 * @see Packet
 * @see PACKET_QUEUE_SIZE
 * @see crc32Buffer
 */
void processIncomingPacket(int numBytes) // Must be called from ISR
{
    Serial.printf("Push Packet: head=%d, tail=%d\n", queueHead, queueTail);
    int nextTail = (queueTail + 1) % PACKET_QUEUE_SIZE;
    if (nextTail == queueHead)
    {
        Serial.println("Packet queue overflow");
        return;
    }

    Packet *packet = &(packetQueue[queueTail]);
    uint8_t *ptr = (uint8_t *)packet;
    size_t bytesRead = 0;
    while (Wire.available() && bytesRead < numBytes)
    {
        ptr[bytesRead++] = Wire.read();
    }
    if(bytesRead < sizeof(Packet))
    {
        Serial.printf("Incomplete packet received. Wanted %d bytes, got %d bytes\n", sizeof(Packet), bytesRead);
        return;
    }
    if(bytesRead > sizeof(Packet))
    {
        Serial.printf("Packet too large. Wanted %d bytes, got %d bytes\n", sizeof(Packet), bytesRead);
        Wire.flush(); // Clear remaining bytes
        return;
    }   
    uint32_t checksum = crc32Buffer((uint8_t*) packet, sizeof(Packet) - 4);
    if (checksum != ntohl(packet->checksum))
    {
        Serial.printf("Invalid packet checksum. Expected 0x%08X, got 0x%08X, size %d\n", ntohl(packet->checksum), checksum, sizeof(Packet));
    }
    queueTail = nextTail;
}

The "packets.h" file just contains the Packet struct. But as I said, no rush at all!
 
Anything that can receive some I2C message.

No. I'm not going to begin an investigation if I have to substitute something else which isn't confirmed to reproduce the problem. I've gone down that path many times before. It almost always turns out to be fruitless wasted effort.

Even if a complete list of all the exact hardware, and if any of the other hardware requires programming complete copies of the code needed for every programmable item, I also need a photo of your wiring and clear picture of how all power and ground are really connected. As we've seen on other conversations, problems with power disruption and ground loops are often the cause of clicks or other small audio problems, so this info which may seem excessive really is necessary.

If we do have some sort of software problems between Wire library and Audio in I2S slave mode, I do want to get to the bottom of it. But I'm not going to even begin without clear and complete info to reliably reproduce the problem.
 
Last edited:
I think I figured it out. Hardware issues. The I2C and I2S pins are right next to each other and my messy breadboard construction seems to generate some crosstalk. I can make it worse if I bring the wires close together.
 
Back
Top