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

Reduce stack usage to help usage on embedded systems #117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chmorgan
Copy link

@chmorgan chmorgan commented Oct 6, 2023

No description provided.

…ack consumption by ~16k

mp3dec_s can be placed in the memory type of choosing by the caller but in particular
this helps embedded use where stacks are often fixed sized.
@vmsh0
Copy link

vmsh0 commented Aug 14, 2024

This is useful to me. It would be nice if it would be merged.

@chmorgan
Copy link
Author

@vmsh0 feel free to use my fork and let me know how it goes. I'm using it here on stm32 targets.

@vmsh0
Copy link

vmsh0 commented Aug 15, 2024

I swapped this in in place of the mainline version, was able to reduce the stack size of my decoder task by more than 20k. Big success! Thanks.

@manxorist
Copy link
Contributor

While I would also like to see reduced stack usage, the approach implemented here does not really work:
When using minimp3 in a separate translation unit (i.e. without MINIMP3_IMPLEMENTATION defined), mp3dec_t is now an incomplete type that cannot be used at all, which makes the whole API unusable.

@vmsh0
Copy link

vmsh0 commented Aug 15, 2024

While I would also like to see reduced stack usage, the approach implemented here does not really work: When using minimp3 in a separate translation unit (i.e. without MINIMP3_IMPLEMENTATION defined), mp3dec_t is now an incomplete type that cannot be used at all, which makes the whole API unusable.

The API could be extended to have an "init" method, dynamically allocating a mp3dec_t structure. This would probably benefit from #114

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.

3 participants