Ladspa deduplicate

[bmatherly]

I stumbled across this while investigating something else. The ladspa.* filters are registered twice. The second registration overwrites the first. In the current framework, if a duplicate service is registered, it overwrites the first. This change adds a warning for that situation and avoids the duplicate registration for ladspa.

There are also duplicate registrations in openfx that will be handled in a different PR.

[Copilot]

Pull request overview

This PR prevents duplicate LADSPA service registrations between the mltjackrack and mltladspa modules and adds diagnostics when any service is registered more than once, aligning service registration behavior with the current “last registration wins” repository behavior.

Changes:

• Split jackrack/factory.c registrations so ladspa* services are only registered from the mltladspa module, avoiding overwrite-by-duplicate.
• Add duplicate-registration detection in mlt_repository_register() and emit a log message when overwriting an existing service registration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/modules/jackrack/factory.c	Guards LADSPA vs JACK service registration by module export macro to prevent duplicate registrations.
src/framework/mlt_repository.c	Detects duplicate registrations and logs when a registration overwrites a prior one.

[ddennedy]

Duplication is intentional;-see f50567b
Does this change break that? What problem does redundant registration cause? I agree to change mlt_repository to ignore a duplicate registration but not log it at error level, only perhaps debug level.

[bmatherly]

Duplication is intentional. Does this change break that?

The current behavior is that mltladspa and mltjackrack library files both register all of the LADSPA plugins. The second registration replaces the first.

This proposed change makes it so that mltladspa loads the plugins and mltjackrack does not.

Is that broken? I don't understand why mltjackrack would need to load the ladspa plugins if ladspa is already doing it.

My goal is to avoid unintentional double registration as is happening in openfx. So I like the error message as a way to alert a developer if they make a mistake. But if there are good reasons to duplicate registrations, then maybe this change is not helpful.

Another approach I had considered: keep the error message, but allow a module to query to see if a service exists before registering it. I would be happy to add that for ladspa if duplicate registration is useful for that module.

[ddennedy]

Is that broken? I don't understand why mltjackrack would need to load the ladspa plugins if ladspa is already doing it.

I am fairly sure you are correct. You can build mltjackrack with JACK alone or with LADSPA, but if you build it with LADSPA you also get mltladspa.

[ddennedy]

Now, WITH_JACK is dependent upon the dependency and only defined when the optional module is built. But factory.c is now using the defines mltjackrack_EXPORTS instead of WITH_JACK that I asked for. I said I only wanted to use the EXPORTS defines for controlling the export header includes. I also, asked for the mltladspa module to be optional and define a new WITH_LADSPA that can be used by itself in factory.c instead of defined(mltladspa_EXPORTS) && defined(GPL). The new define depends on (GPL AND TARGET PkgConfig::xml AND TARGET PkgConfig::glib AND ladspa_h_FOUND) So, there would be no need to repeat the depenency on GPL in factory.c. Instead, it would be clean, readable #ifdef WITH_JACK and #ifdef WITH_LADSPA blocks.

[bmatherly]

I was using the *_EXPORTS definition as a way to determine which library was being compiled. I see what you mean about WITH_LADSPA. I will make a commit to do that instead.

[bmatherly]

Can you explain more what you mean by:

I also, asked for the mltladspa module to be optional

Do you want the user to be able to enable/disable mltladspa as a cmake variable when compiling? Or do you just mean that if ladspa_h_FOUND=False, then do not build mltladspa?

We currently have a CMAKE variable to disable the entire jackrack module (directory) - which disables both mltjackrack and mltladspa. Maybe you are suggesting two CMAKE variables - one to disable jackrack and one to disable mltladspa?

[ddennedy]

I previously mentioned that it is possible to build a mltjackrack with only a JACK consumer only mltjackrack, meaning no mltladspa.

I was not asking for a new CMake variable, but we can There is already a line if(GPL AND TARGET PkgConfig::xml AND TARGET PkgConfig::glib AND ladspa_h_FOUND) that defines how to build a mltladspa. That can add the mltladspa library and define WITH_LADSPA. But now, I see it might be possible to build mltladspa without LADSPA but including LV2 and VST2. However, I am not sure that can actually work. So, there can be a new variable USE_LADSPA and to add mltladspa:
if(GPL AND (USE_LADSPA OR USE_LV2 OR USE_VST2))

I will do it and push a commit here.