obs-vst: Prevent crash loading VST plugins with many channels

essej commented 3 years ago

Description

When VST plugins that contained many channels or multiple buses were loaded, OBS would crash. This fix allows them to load and run properly.

This requires corresponding pull request https://github.com/obsproject/obs-studio/pull/3744 in the obs-studio repository that fixes initialization of the audio data pointers being passed in here, in order to function properly.

Motivation and Context

Fixes crashing issues when attempting to use VST plugins that have many channels specifically.

How Has This Been Tested?

This has been tested on macOS 10.15.6 using CoreAudio, so although the change is minimal and safe, it should still be tested on the other platforms. Several commercial plugins that have multiple output buses were tested that previously crashed OBS when loading.

Types of changes

Bug fix

Checklist:

[X] My code has been run through clang-format.
[X] I have read the contributing document.
[X] My code is not on the master branch.
[X] The code has been tested (on Mac)
[X] All commit messages are properly formatted and commits squashed where appropriate.
[X] I have included updates to all appropriate documentation.

essej commented 3 years ago

Take 2, this is a more correct solution, and also fixed a related cleanup bug as well.

walker-WSH commented 2 years ago

This crash can be reproduced with "reastream-standalone-32.dll" on Windows. Solution in this pull request is same with Audacity.

Good job!

walker-WSH commented 2 years ago

My VS2019 reports compile error for: _std::max((sizet)0, count)

Suggest to be: (std::max)((size_t)0, count)

WizardCM commented 2 years ago

Hi there, there are merge conflicts.

walker-WSH commented 2 years ago

Will you please resolve the conflict and compile error? Otherwise, I can create another PR to do it.

WizardCM commented 2 years ago

I will see what I can do and let you know.

WizardCM commented 2 years ago

I've force pushed an update to the branch with a rebase to latest master, and a fix which ensures that it builds on Windows/MSVC.

In my testing, this doesn't introduce any new issues with existing VSTs. I don't currently have any VSTs installed that crash with too many channels, so that's next on my list to test.

essej commented 2 years ago

Hmm, those sizeof()s inside the mallocs are incorrect. They should be using the original (float *) and (float) respectively.

RytoEX commented 2 years ago

Hmm, those sizeof()s inside the mallocs are incorrect. They should be using the original (float *) and (float) respectively.

As the PR author, you are able to make and push those changes. Could you do that?

As an aside, this PR description states that it requires obsproject/obs-studio#3744. However, that PR was closed manually by commit https://github.com/obsproject/obs-studio/commit/9ef76e1f9bd984332d1c04f515be5ee7cb403b4d. Is that commit sufficient for this PR's needs?

walker-WSH commented 2 years ago

if (maxchans < 0 || maxchans > 256)

better to be: if (minChans <= 0 || maxchans > 256)

obsproject / obs-vst