Skip to content
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.

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

Shift I2S input data by 1 bit #2121

Merged
merged 1 commit into from
Apr 24, 2024
Merged

Shift I2S input data by 1 bit #2121

merged 1 commit into from
Apr 24, 2024

Conversation

earlephilhower
Copy link
Owner

Fixes #2037

@frohro
Copy link

frohro commented Apr 24, 2024

@earlephilhower I am happy to report you fixed the issue perfectly according to my test. Just in case I made a mistake here is what I did. There are parts of this I have never done before.

  1. I changed the platformio.ini to include this line: platform_packages =
    framework-arduinopico@https://github.com/earlephilhower/arduino-pico.git#i2sin1
  2. I connected my pico w and built and uploaded with the -> button in VScode PlatformIO. It took a long time reconfiguring the PlatformIO framework but finally finished.
  3. Then I changed my code so it was as we thought it should be, shifting 8 bits for 32 bit encoded packets or 0 bits if I am using 24 bit encoded packets. That now looks like this:

void BufferFiller::fillBuffer()
{
    while (!mutex_try_enter(&my_mutex, &mutex_save))
    {
        // Mutex is locked, so wait here.
    }
    char *buffer = queue.getNextBufferAndUpdate(true);
    mutex_exit(&my_mutex);
    if (buffer != nullptr)
    {
        static int32_t r, l, packet_number = 0;
        uint32_t bufferIndex = 4;
        while (bufferIndex < BUFFER_SIZE - 4) // Leave space for the packet number
        {
            i2s.read32(&l, &r);
            if (BITS_PER_SAMPLE_SENT == 24)
            {
                // l = l << 1;
                // r = r << 1;
                l &= 0xFFFFFF;                       // Keep only the lower 24 bits
                r &= 0xFFFFFF;                       // Keep only the lower 24 bits
                memcpy(buffer + bufferIndex, &l, 3); // Copy only the lower 3 bytes
                bufferIndex += 3;
                memcpy(buffer + bufferIndex, &r, 3); // Copy only the lower 3 bytes
                bufferIndex += 3;
            }
            else if(BITS_PER_SAMPLE_SENT == 32)
            {
                // l = l << 9;
                // r = r << 9;
                l = l << 8; // Keep only the lower 24 bits
                r = r << 8; // Keep only the lower 24 bits
                memcpy(buffer + bufferIndex, &l, sizeof(int32_t)); // Copy only the lower 3 bytes
                bufferIndex += sizeof(int32_t);
                memcpy(buffer + bufferIndex, &r, sizeof(int32_t)); // Copy only the lower 3 bytes
                bufferIndex += sizeof(int32_t);
            }
        }
        memcpy(buffer, &packet_number, sizeof(int32_t));  
        packet_number++;
        Serial.printf("Filled %d\n", packet_number);
    }
}
  1. I sent it via UDP to my Linux box, received and plotted it there using a python script that was working just fine with the extra bit shift and old framework. The plot looks like this:
    image
    and this is what I was expecting, as I have a sinewave at 1 kHz and 2 V amplitude feeding both left and right channels.
    That was the test.

I would appreciate your advice about when I should change frameworks, and what I should change to.

Thank you so much for fixing this bug for me!!

Rob

@earlephilhower earlephilhower merged commit bd8eb9b into master Apr 24, 2024
13 checks passed
@earlephilhower earlephilhower deleted the i2sin1 branch April 24, 2024 18:20
@earlephilhower
Copy link
Owner Author

Thanks for the update! With the waveform captured it was pretty obvious what was going on.

You should probably use the git version (I think you need to just drop the #xxx) until the next release to get this minor patch.

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

Successfully merging this pull request may close these issues.

PCM1808 Does Not Work Correctly Using I2S Unless I Shift the Result by Nine Bits. I Expected Eight.
2 participants