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

HW/DVDInterface: Avoid heap allocation in DTK callback. #11655

Merged
merged 2 commits into from Mar 14, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

This should be equivalent, and 672 bytes is small enough that having it on the stack should be fine.

Copy link
Member

@delroth delroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment, because I don't think there's a good reason to leave the risk of overflowing a stack buffer here.

Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
@AdmiralCurtiss
Copy link
Contributor Author

tbqh this still seems a bit shaky, this definitely breaks if m_pending_samples ever happens to be not divisible by SAMPLES_PER_BLOCK or audio_data.size() is ever not divisible by ONE_BLOCK_SIZE. This is also the case in master, but maybe worth refactoring anyway...

@AdmiralCurtiss
Copy link
Contributor Author

I've rewritten the math to work in terms of DTK data blocks instead of sample counts, this should be safer in case we ever get a weird value somehow. Please verify.

Copy link
Member

@delroth delroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced moving to blocks made things any simpler... but whatever, either way is fine.

Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
@delroth delroth merged commit e83b6e1 into dolphin-emu:master Mar 14, 2023
14 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the dtk-heap branch March 14, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants