Addressing @ddennedy review comments

- handle the Microsoft Visual C++ compiler in module CMakeLists.txt
- Change every copyright to the year 2025
- Use C99 variables in the for loop initializer declare style
- Clang format and include <stdbool.h> explicitly
- specify which versions of OpenFX supported
- mention OpenFX header files URL in mlt_openfx.c
- using mlt_image and its functions in src/modules/openfx/filter_openfx.c

Author: mr.fantastic

src/modules/openfx/CMakeLists.txt

@@ -8,9 +8,14 @@ add_custom_target(Other_openfx_Files SOURCES
   filter_openfx.yml
 )
 
+include(GenerateExportHeader)
+generate_export_header(mltopenfx)
 target_compile_options(mltopenfx PRIVATE ${MLT_COMPILE_OPTIONS})
-
-target_link_libraries(mltopenfx PRIVATE mlt m PkgConfig::glib ${CMAKE_DL_LIBS})
+target_include_directories(mltopenfx PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
+target_link_libraries(mltopenfx PRIVATE mlt PkgConfig::glib ${CMAKE_DL_LIBS})
+if(NOT MSVC)
+  target_link_libraries(mltopenfx PRIVATE m)
+endif()
 
 set_target_properties(mltopenfx PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${MLT_MODULE_OUTPUT_DIRECTORY}")
 

src/modules/openfx/factory.c

@@ -1,6 +1,6 @@
 /*
  * factory.c -- the factory method interfaces
- * Copyright (C) 2024 Meltytech, LLC
+ * Copyright (C) 2025 Meltytech, LLC
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -19,6 +19,7 @@
 
 #include "mlt_openfx.h"
 #include <glib.h>
+#include <stdbool.h>
 extern OfxHost MltOfxHost;
 static OfxSetHostFn ofx_set_host;
 static OfxGetPluginFn ofx_get_plugin;
@@ -63,7 +64,8 @@ static const char *getArchStr()
 #define OFX_DIRSEP "/"
 #include <dirent.h>
 
-#elif defined(WINDOWS) || defined(WIN32) || defined(_WIN32) || defined(__MINGW32__) || defined(__MINGW64__)
+#elif defined(WINDOWS) || defined(WIN32) || defined(_WIN32) || defined(__MINGW32__) \
+    || defined(__MINGW64__)
 #define OFX_DIRLIST_SEP_CHARS ";"
 #if defined(_WIN64) || defined(__MINGW64__)
 #define OFX_ARCHSTR "Win64"
@@ -89,8 +91,7 @@ static void plugin_mgr_destroy(mlt_properties p)
 {
     int cN = mlt_properties_count(mltofx_context);
 
-    int j;
-    for (j = 0; j < cN; ++j) {
+    for (int j = 0; j < cN; ++j) {
         char *id = mlt_properties_get_name(mltofx_context, j);
         mlt_properties pb = (mlt_properties) mlt_properties_get_data(mltofx_context, id, NULL);
 
@@ -110,8 +111,7 @@ static void plugin_mgr_destroy(mlt_properties p)
 
     int N = mlt_properties_get_int(p, "N");
 
-    int i;
-    for (i = 0; i < N; ++i) {
+    for (int i = 0; i < N; ++i) {
         char tstr[12] = {
             '\0',
         };
@@ -161,14 +161,15 @@ MLT_REPOSITORY
     MltOfxHost.host = (OfxPropertySetHandle) mlt_properties_new();
     mltofx_init_host_properties(MltOfxHost.host);
 
-    char *dir, *openfx_path = getenv("OFX_PLUGIN_PATH"),
-      *load_unsupported_plugins = getenv("MLT_OFX_LOAD_UNSUPPORTED_PLUGINS"); /* Load unsupported plugins for debugging purposes */
+    char *dir,
+        *openfx_path = getenv("OFX_PLUGIN_PATH"),
+        *load_unsupported_plugins = getenv(
+            "MLT_OFX_LOAD_UNSUPPORTED_PLUGINS"); /* Load unsupported plugins for debugging purposes */
 
     bool is_load_unsupported_plugins = false;
-    if (load_unsupported_plugins)
-      {
-	is_load_unsupported_plugins = strcmp(load_unsupported_plugins, "true") == 0 ? true : false;
-      }
+    if (load_unsupported_plugins) {
+        is_load_unsupported_plugins = strcmp(load_unsupported_plugins, "true") == 0;
+    }
 
     size_t archstr_len = strlen(OFX_ARCHSTR);
 
@@ -177,9 +178,9 @@ MLT_REPOSITORY
 
     if (openfx_path) {
         int dli = 0;
-        char *saveptr, *strptr;
+        char *saveptr;
 
-        for (strptr = openfx_path;; strptr = NULL) {
+        for (char *strptr = openfx_path;; strptr = NULL) {
             dir = strtok_r(strptr, MLT_DIRLIST_DELIMITER, &saveptr);
             if (dir == NULL)
                 break;
@@ -247,25 +248,23 @@ MLT_REPOSITORY
                     if (ofx_get_plugin == NULL)
                         goto parse_error;
 
-                    int i;
-                    for (i = 0; i < NumberOfPlugins; ++i) {
+                    for (int i = 0; i < NumberOfPlugins; ++i) {
                         OfxPlugin *plugin_ptr = ofx_get_plugin(i);
 
                         char *s = NULL;
                         size_t pluginIdentifier_len = strlen(plugin_ptr->pluginIdentifier);
                         s = malloc(pluginIdentifier_len + 8);
                         sprintf(s, "openfx.%s", plugin_ptr->pluginIdentifier);
 
-			/* if colon `:` exists in plugin identifier
+                        /* if colon `:` exists in plugin identifier
 			   change it to accent sign `^` because `:`
 			   can cause issues with mlt if put in filter
 			   name */
-			char *str_ptr = strchr(s, ':');
-			while (str_ptr != NULL) {
-			  *str_ptr++ = '^';
-			  str_ptr = strchr(str_ptr, ':');
-			}
-
+                        char *str_ptr = strchr(s, ':');
+                        while (str_ptr != NULL) {
+                            *str_ptr++ = '^';
+                            str_ptr = strchr(str_ptr, ':');
+                        }
 
                         mlt_properties p;
                         p = mlt_properties_new();
@@ -279,20 +278,20 @@ MLT_REPOSITORY
                         mlt_properties_set(p, "dli", dl_n);
                         mlt_properties_set_int(p, "index", i);
 
-			/* Sometimes error codes other than kOfxStatErrMissingHostFeature
+                        /* Sometimes error codes other than kOfxStatErrMissingHostFeature
 			   returned from kOfxActionDescribe like kOfxStatErrMemory, kOfxStatFailed, kOfxStatErrFatal */
-			bool plugin_supported = mltofx_is_plugin_supported(plugin_ptr) == kOfxStatOK;
+                        bool plugin_supported = mltofx_is_plugin_supported(plugin_ptr)
+                                                == kOfxStatOK;
                         /* WIP: this is only creating them as filter I should find a way to see howto detect producers
 			   if they exists in OpenFX plugins
 			*/
-			if (plugin_supported || is_load_unsupported_plugins)
-			  {
-			    MLT_REGISTER(mlt_service_filter_type, s, filter_openfx_init);
-			    MLT_REGISTER_METADATA(mlt_service_filter_type,
-						  s,
-						  metadata,
-						  "filter_openfx.yml");
-			  }
+                        if (plugin_supported || is_load_unsupported_plugins) {
+                            MLT_REGISTER(mlt_service_filter_type, s, filter_openfx_init);
+                            MLT_REGISTER_METADATA(mlt_service_filter_type,
+                                                  s,
+                                                  metadata,
+                                                  "filter_openfx.yml");
+                        }
                     }
 
                 parse_error:
@@ -304,7 +303,7 @@ MLT_REPOSITORY
                 de = readdir(d);
             }
 
-	    closedir(d);
+            closedir(d);
         }
 
         mlt_properties_set_int(mltofx_dl, "N", dli);

src/modules/openfx/filter_openfx.c

@@ -1,6 +1,6 @@
 /*
  * filter_openfx.c -- filter Video through OpenFX plugins
- * Copyright (C) 2024 Meltytech, LLC
+ * Copyright (C) 2025 Meltytech, LLC
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -41,16 +41,15 @@ static int filter_get_image(mlt_frame frame,
     mlt_properties params = mlt_properties_get_data(image_effect, "mltofx_params", NULL);
     mlt_properties image_effect_params = mlt_properties_get_data(image_effect, "params", NULL);
 
-    *format = mlt_image_rgba;
+    format = mlt_image_rgba;
     int error = mlt_frame_get_image(frame, image, format, width, height, 1);
 
     if (error == 0) {
         mlt_position position = mlt_filter_get_position(filter, frame);
         mlt_position length = mlt_filter_get_length2(filter, frame);
         int params_count = mlt_properties_count(params);
 
-        int i;
-        for (i = 0; i < params_count; ++i) {
+        for (int i = 0; i < params_count; ++i) {
             char *iprop_name = mlt_properties_get_name(params, i);
 
             char *mlt_value = mlt_properties_get(properties, iprop_name);
@@ -59,19 +58,21 @@ static int filter_get_image(mlt_frame frame,
                 mlt_properties param = mlt_properties_get_data_at(params, i, NULL);
 
                 char *type = mlt_properties_get(param, "type");
-		char *widget = mlt_properties_get(param, "widget");
+                char *widget = mlt_properties_get(param, "widget");
 
                 if (type != NULL) {
-		    if (widget != NULL && (strcmp(widget, "2dpoint") == 0 || strcmp(widget, "2dsize") == 0) && strcmp(type, "double") == 0) {
-		        mlt_rect value = mlt_properties_anim_get_rect(properties,
-								     iprop_name,
-								     position,
-								     length);
-			mltofx_param_set_value(image_effect_params,
+                    if (widget != NULL
+                        && (strcmp(widget, "2dpoint") == 0 || strcmp(widget, "2dsize") == 0)
+                        && strcmp(type, "double") == 0) {
+                        mlt_rect value = mlt_properties_anim_get_rect(properties,
+                                                                      iprop_name,
+                                                                      position,
+                                                                      length);
+                        mltofx_param_set_value(image_effect_params,
                                                iprop_name,
                                                mltofx_prop_double2d,
                                                value);
-		    } else if (strcmp(type, "double") == 0) {
+                    } else if (strcmp(type, "double") == 0) {
                         double value = mlt_properties_anim_get_double(properties,
                                                                       iprop_name,
                                                                       position,
@@ -88,12 +89,13 @@ static int filter_get_image(mlt_frame frame,
                                                mltofx_prop_int,
                                                value);
                     } else if (strcmp(type, "string") == 0) {
-		        int value
-			    = mlt_properties_anim_get_int(properties, iprop_name, position, length);
-			mltofx_param_set_value(image_effect_params,
-					       iprop_name,
-                                               mltofx_prop_int, /* for handling option choice TODO: do something better */
-					       value);
+                        int value
+                            = mlt_properties_anim_get_int(properties, iprop_name, position, length);
+                        mltofx_param_set_value(
+                            image_effect_params,
+                            iprop_name,
+                            mltofx_prop_int, /* for handling option choice TODO: do something better */
+                            value);
                     } else if (strcmp(type, "boolean") == 0) {
                         int value
                             = mlt_properties_anim_get_int(properties, iprop_name, position, length);
@@ -102,41 +104,52 @@ static int filter_get_image(mlt_frame frame,
                                                mltofx_prop_int,
                                                value);
                     } else if (strcmp(type, "color") == 0) {
-		        mlt_color value
-			          = mlt_properties_anim_get_color(properties, iprop_name, position, length);
-		        mltofx_param_set_value(image_effect_params,
-					       iprop_name,
-					       mltofx_prop_color,
-					       value);
-		    }
+                        mlt_color value = mlt_properties_anim_get_color(properties,
+                                                                        iprop_name,
+                                                                        position,
+                                                                        length);
+                        mltofx_param_set_value(image_effect_params,
+                                               iprop_name,
+                                               mltofx_prop_color,
+                                               value);
+                    }
                 }
             }
         }
 
         mltofx_begin_sequence_render(plugin, image_effect);
 
-	/* According to OpenFX documentation: Note that hosts that
+        /* According to OpenFX documentation: Note that hosts that
 	   have constant sized imagery need not call this action, only
 	   hosts that allow image sizes to vary need call this. */
-	/* mltofx_get_region_of_definition(plugin, image_effect); */
+        /* mltofx_get_region_of_definition(plugin, image_effect); */
 
         mltofx_get_regions_of_interest(plugin, image_effect, (double) *width, (double) *height);
         mltofx_get_clip_preferences(plugin, image_effect);
 
-        uint8_t *src_copy = malloc(*width * *height * 4);
-        if (src_copy == NULL)
-            goto out;
-        memcpy(src_copy, *image, *width * *height * 4);
+        struct mlt_image_s src_img;
+        mlt_image_set_values(&src_img, *image, *format, *width, *height);
+
+        struct mlt_image_s src_img_copy;
+        mlt_image_set_values(&src_img_copy, NULL, *format, *width, *height);
+
+        mlt_image_alloc_data(&src_img_copy);
+
+        uint8_t *src_copy = src_img_copy.data;
+
+        memcpy(src_copy, *image, mlt_image_calculate_size(&src_img));
         mltofx_set_source_clip_data(plugin, image_effect, src_copy, *width, *height);
         mltofx_set_output_clip_data(plugin, image_effect, *image, *width, *height);
 
+        mlt_service_lock(MLT_FILTER_SERVICE(filter));
         mltofx_action_render(plugin, image_effect, *width, *height);
+        mlt_service_unlock(MLT_FILTER_SERVICE(filter));
 
-        free(src_copy);
+        mlt_image_close(&src_img_copy);
 
         mltofx_end_sequence_render(plugin, image_effect);
     }
-out:
+
     return error;
 }
 

src/modules/openfx/filter_openfx.yml

@@ -9,4 +9,4 @@ url: https://openeffects.org/
 creator: mr.fantastic <mrfantastic@firemail.cc>
 tags:
   - Video
-description: Process videos using OpenFX plugins.
+description: Process videos using OpenFX 1.5 plugins.