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

Check active state before querying latency. Fixes #229 #230

Conversation

Schroedingers-Cat
Copy link
Contributor

The VST3 wrapper simply forwards the host's latency request without checking whether the plugin is active. This causes a misbehaving host log when using the CLAP helpers (and is not allowed by CLAP's specs).

This PR fixes that problem by checking the plugin's active state before getting its latency. This PR also fixes #229.

@baconpaul
Copy link
Collaborator

This looks good to me but I think we should merge it into next then cherry pick it to main. But @defiantnerd i am unsure how next v main is tracking right now.

@defiantnerd
Copy link
Collaborator

I need to think about it - it avoids the protocol violation, but might lead to strange behavior with a wrapped host if it does not ask again for the latency.

@Schroedingers-Cat
Copy link
Contributor Author

but might lead to strange behavior with a wrapped host if it does not ask again for the latency.

Even without this fix, the acquired latency will be of questionable correctness since the samplerate isn't known before the plugin is active.

I can update this PR next Monday with a behaviour that stores a missed latency call and handles that in the next activation of the plugin. That should cover the scenario of a plugin not reporting the latency on its own.

@baconpaul
Copy link
Collaborator

definitely if you ask an inactive plugin for latency you could queue a latency change update after the active

have we considered just activating the plugin here though also? what's the call path by which this happens?

@defiantnerd
Copy link
Collaborator

would this also be useful for the tail-extension?

 auto tailsize = this->_plugin->_ext._tail->get(_plugin->_plugin);

@defiantnerd
Copy link
Collaborator

is it possible that the necessary values (samplerate/blocksize) have been already set and only the setActive() call is missing and the host assumes that the (vst3-)plugin might now be able to tell the correct latency?

@Schroedingers-Cat
Copy link
Contributor Author

Schroedingers-Cat commented Feb 26, 2024

have we considered just activating the plugin here though also? what's the call path by which this happens?

It's a direct call from the host (Reaper in my case, but Cubase 13 is also reported to instantly crash).

I've updated the PR so that a missed latency-call is stored and acted upon during setActive(). The changes hit an issue with the format checks but no error message is being printed. Any idea why?

EDIT: Seems my Xcode produced weird indentations. Fixed that failing check by reformatting the changed code with VS.

@Schroedingers-Cat Schroedingers-Cat force-pushed the bugfix/check-active-on-get-latency branch 2 times, most recently from d830f2c to 02a9b9d Compare February 26, 2024 12:06
@Schroedingers-Cat Schroedingers-Cat force-pushed the bugfix/check-active-on-get-latency branch from 02a9b9d to 45b69bc Compare February 26, 2024 12:11
@baconpaul
Copy link
Collaborator

This looks good to me. @defiantnerd shall we include this in 0.7.2 also along with your other changes? If so I can do the double merge and move and stuff.

@Schroedingers-Cat
Copy link
Contributor Author

Schroedingers-Cat commented Feb 26, 2024

I've added three additional commits in correspondence with @defiantnerd that fix additional crashes during the initialization phase (Reaper, Nuendo). I have no idea how I didn't have these issues last week on my Intel Mac, but these problems were happening on both my Win system and ARM Mac as well as on @defiantnerd 's Win system.

I'd recommend merging this PR instead of going forward with the incomplete merge via 06e49cf as it's missing another misbehavior fix (bbe9f89) as well as the requested change to store a missed latency call (45b69bc).

@defiantnerd
Copy link
Collaborator

it seems that the vst3 documentation contradicts itself and getLatency() is allowed between setupProcessing and setActive - I heard that at least Reaper and Nuendo are doing it that way. So we need to solve this differently by introducing a feature where the CLAP is being transitioned between the necessary states.

@defiantnerd
Copy link
Collaborator

I'll merge this now. We'll give it a makeover later.

@defiantnerd defiantnerd merged commit c8d35cc into free-audio:main Mar 4, 2024
17 checks passed
defiantnerd added a commit that referenced this pull request Mar 10, 2024
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.

CLAP wrapped VST3 crashing most hosts since CLAP 1.2.0 update
3 participants