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

DEBUG ASSERT (2) in effects_refactoring branch #10582

Closed
mixxxbot opened this issue Aug 23, 2022 · 6 comments
Closed

DEBUG ASSERT (2) in effects_refactoring branch #10582

mixxxbot opened this issue Aug 23, 2022 · 6 comments

Comments

@mixxxbot
Copy link
Collaborator

Reported by: daschuer
Date: 2021-10-23T17:49:06Z
Status: Confirmed
Importance: High
Launchpad Issue: lp1948540


I see this as start up:

critical[Main] DEBUG ASSERT: "m_registeredEffects.contains(effectId)" in function virtual EffectManifestPointer BuiltInBackend::getManifest(const QString&) const at /home/daniel/workspace/i18n/src/effects/backends/builtin/builtinbackend.cpp:91

This happens if an configured effect is no longer enabled for some reasons.

@mixxxbot
Copy link
Collaborator Author

Commented by: Be-ing
Date: 2021-10-23T18:23:24Z


Please elaborate how to reproduce this.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2021-10-25T22:42:41Z


I was able to reproduce it by de-installing an LV2 plugin. However this was nothing I have done in the first place. So I guess there must be something else wrong.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2021-10-29T11:39:20Z


Got it. The issue is that EffectPresetManager::loadDefaultEffectPresets tries to create a preset for every file it finds in the directory. In case of an non existing effect, it has a preset with a null manifest, causing the assertion. This can be fixed by only looking up files for existing effecs.

@mixxxbot
Copy link
Collaborator Author

Commented by: Be-ing
Date: 2021-10-29T12:07:53Z


What is the solution? Log a warning instead of an assertion?

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2021-10-29T12:46:28Z


The issue is that all the sorrounding code is not aware of a failure.
This applies to the string for the backend in the same way.
There we must add some extra null checks and than we can finally remove the assertion.
I didn't not think we need a warning at all.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
@mixxxbot mixxxbot added this to the 2.4.0 milestone Aug 24, 2022
@daschuer
Copy link
Member

This assertion is no longer in the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants