Skip to content

Conversation

@dragonx
Copy link

@dragonx dragonx commented Sep 24, 2025

This PR is a proof of concept, but need some discussion on how to handle this "properly".

The esp32-c3-lyra board has no DAC, but the i2s data pin is connected directly to an audio amplifier.
Using the audiobusio library, I was able to play music on this board, but it sounded terrible.

Comparing the i2s implementation in CircuitPython vs the working sample in the esp-adf dev kit, it turns out that the dev kit isn't using true i2s, but the i2s hardware has a PDM output mode. This prototype forces the i2s output pin into this PDM mode.

The reason it's implemented in audiobusio/I2SOut.c right now is because I started looking at the I2S peripheral, and this file is where the I2S peripheral code is. However, it seems to not belong here for a few reasons: a) it's not really running in I2S mode, b) it's using a single pin, so the audiobusio API that uses multiple pins doesn't make sense.

Against argument a) above though, there is an audiobusio.PDMIn API that takes one pin. It seems like the logical followup would be to have an audiobusio.PDMOut API.

Another alternative that comes to mind is the audiopwmio API, which is a "natural" place for beginners to go, since it's used in a bunch of examples. The API would look the same, except that the name is wrong by one letter. Perhaps adding a flag or PWM vs PDM, or an audiopwmio.PDMAudioOut API.

gychang added 2 commits September 10, 2025 12:12
The I2S data pin is hooked up directly to the amplifier on this board,
so while it does generate sound, it sounds terrible since it's not
actually handling the data signal.  The sample implementation in the
esp-adf uses the I2S hardware with PDM encoding to drive the amp.

https://github.com/espressif/esp-adf/blob/ad4ac707c5dffb450dbabfdcaa7443165ee4e852/examples/player/pipeline_spiffs_mp3/main/play_spiffs_mp3_example.c#L76

This change hardcodes the PDM implementation for this board.
@dragonx
Copy link
Author

dragonx commented Sep 24, 2025

  • This PR builds and I can play audio on the esp32-c3-lyra using the audiomp3 decoder.
  • Other esp32 ports also compile without errors

@tannewt
Copy link
Member

tannewt commented Sep 25, 2025

Another alternative that comes to mind is the audiopwmio API, which is a "natural" place for beginners to go, since it's used in a bunch of examples. The API would look the same, except that the name is wrong by one letter. Perhaps adding a flag or PWM vs PDM, or an audiopwmio.PDMAudioOut API.

This is my preferred approach. The docs already note that the platform gets to decide the frequency of the PWM pulses so we can make it a little less specific to encompass PDM.

@dragonx
Copy link
Author

dragonx commented Sep 25, 2025

Another alternative that comes to mind is the audiopwmio API, which is a "natural" place for beginners to go, since it's used in a bunch of examples. The API would look the same, except that the name is wrong by one letter. Perhaps adding a flag or PWM vs PDM, or an audiopwmio.PDMAudioOut API.

This is my preferred approach. The docs already note that the platform gets to decide the frequency of the PWM pulses so we can make it a little less specific to encompass PDM.

So have some sort of #ifdef or variable in board.c to define that PWMAudioOut uses PDM on this board?

Also I'm just ramping up on this. I couldn't find an audiopwmio in common-hal, should I be looking to implement this in ports/espressif/common-hal/pwmio?

@tannewt
Copy link
Member

tannewt commented Sep 25, 2025

So have some sort of #ifdef or variable in board.c to define that PWMAudioOut uses PDM on this board?

Nope, just implement audiopwmio for Espressif using PDM for everyone.

Also I'm just ramping up on this. I couldn't find an audiopwmio in common-hal, should I be looking to implement this in ports/espressif/common-hal/pwmio?

Yup! Add ports/espressif/common-hal/audiopwmio with the implementation and enable it by setting CIRCUITPY_AUDIOPWMIO=1. I'd suggest using starting with another implementation and clearing out the function bodies. You could try an LLM with your existing changes to see if can mash the two together for you.

@dragonx
Copy link
Author

dragonx commented Sep 26, 2025

@tannewt thanks for the input. I'm not likely going to get to this next week, so it'll be a little while.
I'm not sure if there's any chance anyone else will take a look at this... if so you might want to leave this PR open as a reference. However, since the actual fix will look pretty different in terms of the affected files, you can close this and I'll submit another PR when I get around to this.

@tannewt
Copy link
Member

tannewt commented Sep 26, 2025

you can close this and I'll submit another PR when I get around to this.

Works for me. Thanks!

@tannewt tannewt closed this Sep 26, 2025
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.

2 participants