-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix #279 - Fail to render when non-VST fx are in fx chains #280
Conversation
faa8eb9
to
aee9fd0
Compare
v6.37 supports TrackFX_GetNamedConfigParm( track, 0, "fx_name" ) Some earlier versions support TrackFX_GetNamedConfigParm, but not with the "fx_name" arg Modification is to address this bug in the header: https://forum.cockos.com/showthread.php?p=2548853 (post 4)
aee9fd0
to
b63ba92
Compare
const size_t fxNameMaxLen = 1024; // Should be plenty | ||
char fxName[fxNameMaxLen]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just on 'stackwatch' because of the previous issue - do we call this from anywhere that will be troubled by pushing 1k onto the stack? If so:
const std::size_t fxNameMaxLen = 1023;
// I think the '\0' is superfluous, but it shouldn't hurt.
std::vector<char> fxName(fxNameMaxLen + 1, '\0');
if(auto fxNameRes = TrackFX_GetNamedConfigParm(track, fxNum, "fx_name", fxName.data(), fxNameMaxLen)) {
fxName[fxNameMaxLen] = '\0'; // just in case REAPER is naughty
name = std::string(fxName.data());
} else {
return false;
}
Or if we're doing it a few times
template<typename F>
std::optional<std::string> stringFromAPICall(F&& f, std::size_t maxLength = 1023)
{
std::vector<char> buffer(maxLength + 1, '\0');
if(auto result = f(buffer.data(), maxLength)) {
buffer[maxLength] = '\0';
return std::string(buffer.data());
} else {
return {};
}
}
and then
if(auto name = stringFromAPICall([track, fxNum](char* buffer, std::size_t size) {
return TrackFX_GetNamedConfigParm(track, fxNum, "fx_name", buffer, size);
}) {
// do something with *name
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're safe as the old code used to allocate 64k on the stack for the entire state chunk before parsing it. That said, if you think we should go with the vector for safety anyway, happy to go ahead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, good point. No worries then.
Problem is outlined in detail in #279
The fix chosen was to replace some of our custom functions with the built-in Reaper API function for getting original plugin names in cases where plugins may have been renamed in the DAW.
This API function is only supported in REAPER v6.37+ and so the minimum version for the EPS would jump from v6.11 to v6.37. However, v6.37 is now 3 years old and anyone with a license for v6.11 should also be able to upgrade to any v6.xx version so I don't see this causing issues for many (if any) users.
Closes #279