Allow Teakra to either own DSP memory or have the user provide their own#66
Merged
wwylele merged 4 commits intowwylele:masterfrom Aug 21, 2025
Merged
Allow Teakra to either own DSP memory or have the user provide their own#66wwylele merged 4 commits intowwylele:masterfrom
wwylele merged 4 commits intowwylele:masterfrom
Conversation
Co-Authored-By: TheTurtle <47210458+raphaelthegreat@users.noreply.github.com>
Contributor
Author
|
Err, I should probably make CI run tests on pull request |
wwylele
reviewed
Aug 21, 2025
src/shared_memory.h
Outdated
| struct SharedMemory { | ||
| std::array<u8, 0x80000> raw{}; | ||
| u8* raw; | ||
| bool own_dsp_mem; |
Owner
There was a problem hiding this comment.
I think a better way in modern C++ to do this is something like
std::option<std::unique_ptr<std::array<u8, 0x80000>>> raw;
u8* raw_ptr; // points to either raw or user-supplied memory
so when Teakra manages the memory, it doesn't need to manually do new/delete, and it reduce the chance we mishandle and leak it
Contributor
Author
There was a problem hiding this comment.
Should be fixed. Skipped the optional since by default unique_ptr is initialized to null and doesn't own any memory.
Owner
There was a problem hiding this comment.
You can tell my C++ skills are too rust-y by not thinking about null values 😂
wwylele
approved these changes
Aug 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuing from #55
As the title says, this PR lets the user supply their own DSP memory if needed, for purposes such as implementing fastmem, using host shared memory for exposing DSP RAM to other processing, applying memory protections to DSP memory, and so on.
This is done via a UserConfig object via which the user passes to Teakra. This config object will also be needed when it's time to merge the Teak JIT from https://github.com/raphaelthegreat/teakra/ (for enabling/disabling the JIT), which is probably not going to be any time soon as it needs a hefty rewrite.
One minor caveat is that this UserConfig struct hasn't been ported to the C interface yet, and they behave the same as they did before this PR (ie always owning DSP memory). The reason for this is that I'm unsure how to port the struct in the longterm without duplicating a lot of code (by eg making a C version of the struct, and having the interface's cpp file copy each field to the C++ version). Regardless, this isn't a huge problem since the C interface isn't as widely used, and users likely don't care much about this in the first place. It will be more important to implement C interface configs when the JIT is ready.
Tested locally by integrating the changes to Panda3DS, running the test verifier, and by testing the Rust bindings to see that the C interface still works fine.