Implement PWM audio out for the nrf port - #2000
Conversation
|
I'd be happy to merge this early with bugs but would like it to be in it's own module and class rather than AudioOut. At some point I'd like PWM AudioOut on SAMDs as well for pin flexibility. |
|
@jepler, there are merge conflicts, so it won't run a build until they're resolved. |
|
I've updated the branch, and revised the description of the patch to reflect progress since I filed it. @tannewt |
|
@tannewt While this is unfinished due to pause/resume being missing, I think the renaming and the bulk of the work is done. |
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thank you for the rename! This is very close. One more name suggestion and a build to fix. (Looks like builds that disable audiobusio.) Thanks!
There was a problem hiding this comment.
Could you rename this to PWMAudioOut or PulseAudioOut too? That way it'll print differently than audioio.AudioOut.
There was a problem hiding this comment.
I realize this is super nitpicky but please rename the files and function names too from audioout to pwmaudioout. Future us will thank you for the consistency.
There was a problem hiding this comment.
Does this latest commit get the renames you were looking for, @tannewt ?
Without such a definition, this header is not self-contained, but requires whoever included it to also include vfs_fat.h
This implements AudioOut, with known caveats: * pause/resume are not yet implemented (this is just a bug) * at best, the sample fidelity is 8 bits (this is a hardware limitation) Testing performed: My test system is a Particle Xenon with a PAM8302 op-amp https://www.adafruit.com/product/2130 and 8-ohm speaker. There's no analog filtering between the Xenon's PWM pin and the "A+" input of the amplifier; the "A-" pin is disconnected. It is powered from VUSB. I used pin D4, which is *NOT* listed as a low-speed-only pin, but the code does NOT switch the pin to high drive. This is related to an open issue for general inability to set drive level for pins being used by a "special function" on nrf: adafruit#1270 Nothing about the code I've written should limit the usable pins. All samples I played were 16-bit, generally monophonic at 11025Hz and 22050Hz from the Debian LibreOffice package.
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
One more naming consistency nitpick then it should be good to go. Thanks!
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Yup! Looks good to me. Thanks!
.. and also incidentally fix a problem where a RawSample could only be looped 131070 times.
|
@tannewt I think this is ready for another review. I've updated the initial text of the pull request to reflect the state of testing. |
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Just a couple questions and then it should be good to go.
The original formulation was because I saw the need to avoid a transition from playing to stopped exactly when a resume was taking place. However, @tannewt was concerned about this pause causing trouble, because it could be relatively lengthy (several ms even in a typical case). After reflection, I've convinced myself that updating the registers in this order in resume avoids a window where a "stopped" event can be missed as long as the shortcut is updated first. Testing re-performed: pause/resume testing of looped RawSample and WaveFile audio sources.
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Ok, looks good to me. Thanks!
3 participants
This implements PWMAudioOut. Besides the limitation to 7.5 - 8 bit fidelity, I believe it is now complete.
Testing performed:
My test system is a Particle Xenon with a PAM8302 op-amp https://www.adafruit.com/product/2130 and 8-ohm speaker. There's no analog filtering between the Xenon's PWM pin and the "A+" input of the amplifier; the "A-" pin is disconnected. It is powered from VUSB.
I used pin D4, which is NOT listed as a low-speed-only pin, but the code does NOT switch the pin to high drive. This is related to an open issue for general inability to set drive level for pins being used by a "special function" on nrf: #1270
Nothing about the code I've written should limit the usable pins.
All samples I played were 16-bit, generally monophonic at 11025Hz and 22050Hz from the Debian LibreOffice package.
OK RawSample (8000Hz, 1 channel)
OK carrier frequency 62.5kHz
OK Frequency close to 440Hz (measured via o'scope)
OK Single Play
OK (too short to do stop/playing test)
OK Loop
OK Pause/Resume
OK Stop/Playing
OK WaveFile (11025 and 22050Hz, 1 channel)
OK carrier frequency close to 62.5kHz
OK Duration of sample close to when played on PC
OK Single Play
OK Stop/Playing
OK Loop
OK Pause/Resume
OK Stop/Playing