Make CMake options for modules more strict

[jlskuz]

Right now if you enable a certain module you can't be sure it is really enabled until you read the logs. This is because the module might be disabled later on if a dependency is missing.

Example: -DMOD_FREI0R=ON might still not enable the frei0r module if pkg_check_modules(FREI0R frei0r) does not find frei0r

(On the contrary you can be sure that with -DMOD_FREI0R=OFF a module is really off)

In my opinion this behavior is not good, because it makes it hard to reliably get builds with a certain feature set. I suggest to change this behavior to require the modules dependencies if a module is turned on. This way CMake will fail if the dependencies are missing and the users need to explicitly turn of the module if they don't want to use the module due to the missing dependency.

Using the frei0r module as an example again, the code change would look like this:

 if(MOD_FREI0R)
-  pkg_check_modules(FREI0R frei0r)
-  if(FREI0R_FOUND)
-    list(APPEND MLT_SUPPORTED_COMPONENTS frei0r)
-  else()
-    set(MOD_FREI0R OFF)
-  endif()
+  pkg_check_modules(FREI0R REQUIRED frei0r)
+  list(APPEND MLT_SUPPORTED_COMPONENTS frei0r)
 endif()

What do you think about this suggestion? If you think it makes sense I can go ahead and open a PR

[ddennedy]

I agree.

[bmatherly]

I also agree. That would be a good improvement.

[jlskuz]

Thanks for confirming!

Taking this discussion further there are also some modules that have optional dependencies which do not have a cmake option at all yet.

I think it would make sense to introduce options for them (or just make some of them required?). I am not sure how granular it needs to be and how the options should be named/designed. Some suggestions:

1. As extension to the module, one option: MOD_AVFORMAT_OPTIONAL; MOD_JACKRACK_LV2 or SUBMOD_LV2
2. As extension to the module, many options: MOD_AVFORMAT_AVCODEC, MOD_AVFORMAT_AVFILTER
3. Module independent focused on the dependency: FIND_AVCODEC or USE_AVCODEC or ENABLE_AVCODEC

[ddennedy]

I do not want to establish a policy until we work through some specific cases to help guide the decision. It might not be a universal policy:

1. Regarding FFmpeg, at one point I made it quite flexible, and I do not think it makes sense anymore. See https://www.mltframework.org/docs/buildtips/ for reference (no longer current). The flexibility was to give a distro like Fedora the option to build without libavcodec since it wanted only Free codecs. As I recall, they were building a MLT that would work with libdv, libtheora, libvorbis, and images. But now they do include FFmpeg and have learned how to configure it for the codecs they want. avformat is useless without avcodec. As for excluding devices and filters, why bother and who cares? Let's remove these options.
2. "Module independent focused on the dependency" There are some cases of this now as in fftw. In general, we try to isolate dependencies per module since that is part of the criteria to delineate modules. However, it is not a hard rule. -DENABLE_FFTW=OFF is weird to me because the default should be ON and the word "disable" exists. I like WITH or USE. A Gemini query "What is the most common convention to use as prefix when naming dependency options for cmake?" said WITH for what it's worth.
3. You mentioned LV2. We already have USE_LV2. Logically, it belongs to the jackrack module, and there should be no reason for it to extend to another. MLT_JACKRACK_LV2 makes sense to me.
4. I am working on a change that might add some OpenGL dependencies to the avformat module, which is currently only in Movit. In that case, they would come as a bundle (e.g. epoxy + egl), and I would prefer to group them (both or nothing) as a module option such as MOD_AVFORMAT_OPENGL.

In summary, include the module name as part of the option name if it is really specific to the module. Otherwise, use the prefix WITH_ for a more generic dependency.

[jlskuz]

The JACKRACK module is by far the most complicated one because it has optional dependencies on glib, xml, JACK, ladspa.h and the LV2 and VST2 modules with deps on lilv for LV2. I wonder if we need all that flexibility?

[ddennedy]

glib and libxml2 are required if you are using LADSPA. LV2 or VST2 requires LADSPA.

JACK components can be built without glib, libxml2, and ladspa. I doubt anyone builds this module with only that, but also it is the least complicated. I require the ability to build target mltjackrack with only JACK support (see the most recent commit 50567b.

The least important parts now are for handling JACK Rack XML files, which was its original purpose. That needs glib and libxml2, but the LADSPA (and LV2 and VST2) code is intertwined with that: jackrack requires LADSPA but not the other way around. I will deprecate the jackrack file support, but it has not yet been done. Removal of the jackrack file support will not remove the glib and libxml2 dependencies (without a full module rewrite). Some of the redundancy is intentional to avoid trying to untangle things without trying to rewrite and have a good means to verify results. The mltladspa target is an intentional subset of the mltjackrack target to make JACK optional at runtime, which is important for me to provide a portable build for Linux since I cannot bundle libjack for technical reasons. If you have JACK installed, you get JACK features in addition to LADSPA. If you do not have JACK installed you still get LADSPA.

I would rather you not try to rewrite this, except you can change USE_ to WITH_ and use find_package instead of pkg-config.