Various fixes and improvements for OpenFX filters (#1248)

* Do set unspecified parameters every frame

* Enhance OFX region of definition handling in image effects

* Fix clipGetImage changes const param

* Fix broken RGB(A) (color) handling in paramSetValue

* Fix broken double2D in paramSetValue

* Add support for 2D integer and 3D parameters in paramSetValue

* fix & add 2D and 3D params in paramGetValue

* Add support for 2D integer parameters in openfx filter

* Fix paramSetValue not updating filter-user properties

* NatronParamFormatChoice -> NatronParamFormatSize

Apparently some Natron OpenFX plugins need special handling.

* Fix preview scaling

* Add support for frame threading in OpenFX filters

* Add clip preference management for OpenFX filters

* Implement logging functionality for OpenFX messages

* fix -Werror=unused-variable

* Fix render scale update in serialized mode for OpenFX filters

* Check null image_effect in mltofx_allows_frame_threading

* Fix error handling in paramGetValue

* Fix default value for choice parameters

It was showing as the integer index, not one of the string choices.

Author: ddennedy

CMakePresets.json

@@ -104,7 +104,7 @@
             "binaryDir": "${sourceDir}/build/${presetName}",
             "cacheVariables": {
                 "BUILD_TESTING": "OFF",
-                "CMAKE_INSTALL_PREFIX": "${sourceDir}/install/${presetName}",
+                "CMAKE_INSTALL_PREFIX": "c:/Projects/Shotcut",
                 "CMAKE_BUILD_TYPE": "RelWithDebInfo",
                 "CMAKE_PREFIX_PATH": "C:/Qt/$env{QT_VERSION}/mingw_64;C:/msys64/mingw64;C:/msys64/clangarm64",
                 "MOD_GLAXNIMATE_QT6": "ON",

src/modules/openfx/filter_openfx.c

@@ -19,6 +19,7 @@
 
 #include "mlt_openfx.h"
 
+#include <ctype.h>
 #include <framework/mlt.h>
 #include <ofxImageEffect.h>
 #include <string.h>
@@ -28,6 +29,154 @@ extern mlt_properties mltofx_context;
 extern mlt_properties mltofx_dl;
 
 static const char OFX_IMAGE_EFFECT[] = "_ofx_image_effect";
+static const char NATRON_FORMAT_CHOICE_PARAM[] = "NatronParamFormatChoice";
+static const char NATRON_FORMAT_SIZE_PARAM[] = "NatronParamFormatSize";
+
+typedef struct
+{
+    int saw_choice;
+    int width;
+    int height;
+} natron_format_compat_state;
+
+static int parse_choice_dimensions(const char *choice, int *out_w, int *out_h)
+{
+    if (!choice || !out_w || !out_h)
+        return 0;
+
+    for (const char *p = choice; *p; ++p) {
+        if (!isdigit((unsigned char) *p))
+            continue;
+
+        // Only start at the beginning of a numeric token, not the middle of one.
+        if (p > choice && isdigit((unsigned char) p[-1]))
+            continue;
+
+        const char *q = p;
+        int w = 0;
+        while (isdigit((unsigned char) *q)) {
+            w = w * 10 + (*q - '0');
+            ++q;
+        }
+
+        if (*q != 'x' && *q != 'X')
+            continue;
+        ++q;
+        if (!isdigit((unsigned char) *q))
+            continue;
+
+        int h = 0;
+        while (isdigit((unsigned char) *q)) {
+            h = h * 10 + (*q - '0');
+            ++q;
+        }
+
+        // Height token must also end cleanly (space/end/punctuation), not in the middle of digits.
+        if (isdigit((unsigned char) *q))
+            continue;
+
+        if (w > 0 && h > 0) {
+            *out_w = w;
+            *out_h = h;
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+static void natron_compat_track_choice(natron_format_compat_state *state,
+                                       const char *param_name,
+                                       const char *value)
+{
+    if (!state || !param_name || !value)
+        return;
+    if (strcmp(param_name, NATRON_FORMAT_CHOICE_PARAM) != 0)
+        return;
+
+    state->saw_choice = 1;
+    parse_choice_dimensions(value, &state->width, &state->height);
+}
+
+static void natron_compat_apply_size(mlt_properties image_effect_params,
+                                     const natron_format_compat_state *state)
+{
+    if (!image_effect_params || !state)
+        return;
+    if (!state->saw_choice || state->width <= 0 || state->height <= 0)
+        return;
+
+    mltofx_param_set_value(image_effect_params,
+                           (char *) NATRON_FORMAT_SIZE_PARAM,
+                           mltofx_prop_int2d,
+                           state->width,
+                           state->height);
+}
+
+static void reset_image_effect_action_props(mlt_properties image_effect, const char *name)
+{
+    mlt_properties old_props = mlt_properties_get_properties(image_effect, name);
+    mlt_properties_set_properties(image_effect, name, mlt_properties_new());
+    if (old_props)
+        mlt_properties_close(old_props);
+}
+
+static void init_image_effect_action_props(mlt_properties image_effect)
+{
+    reset_image_effect_action_props(image_effect, "begin_sequence_props");
+    reset_image_effect_action_props(image_effect, "end_sequence_props");
+    reset_image_effect_action_props(image_effect, "get_rod_in_args");
+    reset_image_effect_action_props(image_effect, "get_rod_out_args");
+    reset_image_effect_action_props(image_effect, "get_roi_in_args");
+    reset_image_effect_action_props(image_effect, "get_roi_out_args");
+    reset_image_effect_action_props(image_effect, "get_clippref_args");
+    reset_image_effect_action_props(image_effect, "render_in_args");
+}
+
+static mlt_properties create_image_effect_instance(OfxPlugin *plugin,
+                                                   mlt_properties filter_properties,
+                                                   int sync_param_backlink)
+{
+    mlt_properties params = mlt_properties_new();
+    if (!params)
+        return NULL;
+
+    mlt_properties image_effect = mltofx_fetch_params(plugin, params, NULL);
+    if (!image_effect) {
+        mlt_properties_close(params);
+        return NULL;
+    }
+
+    if (sync_param_backlink) {
+        mlt_properties image_effect_params = mlt_properties_get_properties(image_effect, "params");
+        int param_count = mlt_properties_count(image_effect_params);
+        for (int i = 0; i < param_count; ++i) {
+            char *param_name = mlt_properties_get_name(image_effect_params, i);
+            mlt_properties param = mlt_properties_get_properties(image_effect_params, param_name);
+            if (!param)
+                continue;
+            mlt_properties param_props = mlt_properties_get_properties(param, "p");
+            if (!param_props)
+                continue;
+            mlt_properties_set_data(param_props,
+                                    "_filter_properties",
+                                    filter_properties,
+                                    0,
+                                    NULL,
+                                    NULL);
+        }
+    }
+
+    mltofx_create_instance(plugin, image_effect);
+    init_image_effect_action_props(image_effect);
+    mlt_properties_set_data(image_effect,
+                            "_runtime_params",
+                            params,
+                            0,
+                            (mlt_destructor) mlt_properties_close,
+                            NULL);
+    return image_effect;
+}
 
 static mlt_image_format select_image_format(mlt_image_format incoming,
                                             mltofx_depths_mask plugin_support_depths,
@@ -60,6 +209,8 @@ static void update_plugin_params(mlt_properties properties,
                                  mlt_position position,
                                  mlt_position length)
 {
+    natron_format_compat_state natron_compat = {0};
+
     int params_count = mlt_properties_count(params);
     for (int i = 0; i < params_count; ++i) {
         char *param_key = mlt_properties_get_name(params, i);
@@ -69,6 +220,8 @@ static void update_plugin_params(mlt_properties properties,
         char *param_name = mlt_properties_get(param, "identifier");
         if (!param_name)
             continue;
+        if (!mlt_properties_exists(properties, param_name))
+            continue;
         char *type = mlt_properties_get(param, "type");
         char *widget = mlt_properties_get(param, "widget");
         if (!type)
@@ -77,6 +230,12 @@ static void update_plugin_params(mlt_properties properties,
             && strcmp(type, "float") == 0) {
             mlt_rect value = mlt_properties_anim_get_rect(properties, param_name, position, length);
             mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_double2d, value);
+        } else if (widget && (strcmp(widget, "point") == 0 || strcmp(widget, "size") == 0)
+                   && strcmp(type, "integer") == 0) {
+            mlt_rect value = mlt_properties_anim_get_rect(properties, param_name, position, length);
+            int x = (int) value.x;
+            int y = (int) value.y;
+            mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_int2d, x, y);
         } else if (strcmp(type, "float") == 0) {
             double value = mlt_properties_anim_get_double(properties, param_name, position, length);
             mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_double, value);
@@ -85,14 +244,18 @@ static void update_plugin_params(mlt_properties properties,
             mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_int, value);
         } else if (strcmp(type, "string") == 0) {
             char *value = mlt_properties_anim_get(properties, param_name, position, length);
-            if (value)
+            if (value) {
+                natron_compat_track_choice(&natron_compat, param_name, value);
                 mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_string, value);
+            }
         } else if (strcmp(type, "color") == 0) {
             mlt_color value
                 = mlt_properties_anim_get_color(properties, param_name, position, length);
             mltofx_param_set_value(image_effect_params, param_name, mltofx_prop_color, value);
         }
     }
+
+    natron_compat_apply_size(image_effect_params, &natron_compat);
 }
 
 static int filter_get_image(mlt_frame frame,
@@ -105,9 +268,18 @@ static int filter_get_image(mlt_frame frame,
     mlt_filter filter = (mlt_filter) mlt_frame_pop_service(frame);
     mlt_properties properties = MLT_FILTER_PROPERTIES(filter);
     OfxPlugin *plugin = mlt_properties_get_data(properties, "ofx_plugin", NULL);
-    mlt_properties image_effect = mlt_properties_get_properties(properties, OFX_IMAGE_EFFECT);
+    mlt_properties base_image_effect = mlt_properties_get_properties(properties, OFX_IMAGE_EFFECT);
+    int allow_frame_threading = mlt_properties_get_int(properties, "_allow_frame_threading");
+    mlt_properties image_effect = base_image_effect;
+    if (allow_frame_threading) {
+        image_effect = create_image_effect_instance(plugin, properties, 0);
+        if (!image_effect)
+            image_effect = base_image_effect;
+    }
+
     mlt_properties params = mlt_properties_get_properties(image_effect, "mltofx_params");
     mlt_properties image_effect_params = mlt_properties_get_properties(image_effect, "params");
+    int lock_service = (image_effect == base_image_effect);
 
     mlt_image_format requested_format = *format;
     mltofx_depths_mask plugin_support_depths = mltofx_plugin_supported_depths(image_effect);
@@ -133,6 +305,10 @@ static int filter_get_image(mlt_frame frame,
     if (pixel_aspect_ratio <= 0.0)
         pixel_aspect_ratio = 1.0;
 
+    mlt_profile profile = mlt_service_profile(MLT_FILTER_SERVICE(filter));
+    double render_scale_x = mlt_profile_scale_width(profile, *width);
+    double render_scale_y = mlt_profile_scale_height(profile, *height);
+
     mlt_position position = mlt_filter_get_position(filter, frame);
     mlt_position length = mlt_filter_get_length2(filter, frame);
     double ofx_time = (double) position;
@@ -195,7 +371,11 @@ static int filter_get_image(mlt_frame frame,
                           : use_float ? (uint8_t *) float_out
                                       : *image;
 
-    mlt_service_lock(MLT_FILTER_SERVICE(filter));
+    if (lock_service)
+        mlt_service_lock(MLT_FILTER_SERVICE(filter));
+
+    // In serialized mode, image_effect is shared across frames; update scale under lock.
+    mltofx_set_render_scale(image_effect, render_scale_x, render_scale_y);
 
     update_plugin_params(properties, image_effect_params, params, position, length);
 
@@ -219,8 +399,10 @@ static int filter_get_image(mlt_frame frame,
                                 pixel_aspect_ratio,
                                 ofx_depth);
 
-    // OFX pre-render action order: GetClipPreferences → GetRegionsOfInterest → BeginSequenceRender
+    // OFX pre-render action order:
+    // GetClipPreferences -> GetRegionOfDefinition -> GetRegionsOfInterest -> BeginSequenceRender
     mltofx_get_clip_preferences(plugin, image_effect);
+    mltofx_get_region_of_definition(plugin, image_effect, ofx_time);
     mltofx_get_regions_of_interest(plugin, image_effect, ofx_time, (double) *width, (double) *height);
     mltofx_begin_sequence_render(plugin, image_effect, ofx_time);
 
@@ -245,7 +427,13 @@ static int filter_get_image(mlt_frame frame,
     mltofx_action_render(plugin, image_effect, ofx_time, *width, *height);
     mltofx_end_sequence_render(plugin, image_effect, ofx_time);
 
-    mlt_service_unlock(MLT_FILTER_SERVICE(filter));
+    if (lock_service)
+        mlt_service_unlock(MLT_FILTER_SERVICE(filter));
+
+    if (image_effect != base_image_effect) {
+        mltofx_destroy_instance(plugin, image_effect);
+        mlt_properties_close(image_effect);
+    }
 
     // Convert output back to rgba64 in-place — outside the lock, per-frame only.
     if (use_half) {
@@ -319,30 +507,24 @@ mlt_filter filter_openfx_init(mlt_profile profile, mlt_service_type type, const
         mlt_filter_close(filter);
         return NULL;
     }
-    mlt_properties params = mlt_properties_new();
-    mlt_properties image_effect = mltofx_fetch_params(pt, params, NULL);
-    mltofx_create_instance(pt, image_effect);
-
-    mlt_properties_set_properties(image_effect, "begin_sequence_props", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_rod_in_args"));
-    mlt_properties_set_properties(image_effect, "end_sequence_props", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_rod_out_args"));
-    mlt_properties_set_properties(image_effect, "get_rod_in_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_roi_in_args"));
-    mlt_properties_set_properties(image_effect, "get_rod_out_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_roi_out_args"));
-    mlt_properties_set_properties(image_effect, "get_roi_in_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_roi_in_args"));
-    mlt_properties_set_properties(image_effect, "get_roi_out_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_roi_out_args"));
-    mlt_properties_set_properties(image_effect, "get_clippref_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "get_clippref_args"));
-    mlt_properties_set_properties(image_effect, "render_in_args", mlt_properties_new());
-    mlt_properties_close(mlt_properties_get_properties(image_effect, "render_in_args"));
+    mlt_properties image_effect = create_image_effect_instance(pt, properties, 1);
+    if (!image_effect) {
+        mlt_log_error(filter, "Failed to create OpenFX image effect instance: %s\n", id);
+        mlt_filter_close(filter);
+        return NULL;
+    }
+
+    mlt_properties_set_int(properties,
+                           "_allow_frame_threading",
+                           mltofx_allows_frame_threading(image_effect));
+    mlt_log_info(filter,
+                 "OpenFX threading mode for %s: %s\n",
+                 id,
+                 mlt_properties_get_int(properties, "_allow_frame_threading") ? "frame-threaded"
+                                                                              : "serialized");
     mlt_properties_set_data(properties, "ofx_plugin", pt, 0, NULL, NULL);
     mlt_properties_set_properties(properties, OFX_IMAGE_EFFECT, image_effect);
     mlt_properties_close(image_effect);
-    mlt_properties_close(params);
     filter->process = filter_process;
     filter->close = filter_close;
     return filter;