Skip to content

Remove WaveFile() buffer argument#5228

Closed
dhalbert wants to merge 1 commit into
adafruit:mainfrom
dhalbert:remove-WaveFile-buffer-arg
Closed

Remove WaveFile() buffer argument#5228
dhalbert wants to merge 1 commit into
adafruit:mainfrom
dhalbert:remove-WaveFile-buffer-arg

Conversation

@dhalbert
Copy link
Copy Markdown
Collaborator

This error is about supplying an optional buffer that's split into two for reading WaveFiles, but those buffers is not used for playing unless no conversion is done. Propagating the buffer sizes through is not so easy, and there are some assumptions about using 512-byte buffers. Given that audio is working better, I don't really see a use case for it right now.

The buffer arg is not used by any Learn Guide or library examples.

In the long run, we may be revamping the audio API anyway.

@dhalbert dhalbert requested a review from tannewt August 25, 2021 17:30
@jepler
Copy link
Copy Markdown

jepler commented Aug 25, 2021

@deshipu may want to weigh in too

@deshipu
Copy link
Copy Markdown

deshipu commented Aug 26, 2021

I added the buffer argument to be able to reuse the same pre-allocated buffer, and avoid memory errors stemming from repeated re-allocation of that buffer when playing different sounds repeatedly. I think it's a worthy thing, and rather than removing it, we should make everything use it instead. However, I can't work on this, so it's up to you to decide.

I'm using this in the Vacuum Invaders game, and it has solved a problem of memory fragmentation there, but I am not aware of any other games or other programs that actually use it.

@dhalbert
Copy link
Copy Markdown
Collaborator Author

@deshipu that is worthy. I had forgotten the original motivation of buffer reuse. I see you use a buffer size of 128. People were trying to make the buffer really big to improve audio playback quality, which didn't really work. So I will just throw an error when the size is out of range (>1024). I will make a new PR

@dhalbert dhalbert closed this Aug 26, 2021
@dhalbert dhalbert deleted the remove-WaveFile-buffer-arg branch August 26, 2021 13:03
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.

Supplying a larger buffer for RP2040 PWMAudioOut fails

3 participants