From 26d2d2e01502702cbe512ad4e9d8fbec59287804 Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 15:27:27 -0700 Subject: [PATCH 1/6] Fix fx_cut affected video tracks above it Adds 2 new functions: `mlt_frame_get_image_from_service()` `mlt_frame_get_image_with_fx_cut()` --- NEWS | 5 + src/framework/mlt.vers | 2 + src/framework/mlt_frame.c | 151 ++++++++++++++++++++++ src/framework/mlt_frame.h | 12 ++ src/framework/mlt_playlist.h | 1 + src/framework/mlt_tractor.c | 90 ++----------- src/framework/mlt_transition.c | 80 ++++++++---- src/tests/test_tractor/test_tractor.cpp | 164 +++++++++++++++++++++++- 8 files changed, 391 insertions(+), 114 deletions(-) diff --git a/NEWS b/NEWS index 781804081..402ce2f3f 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,11 @@ Framework longer have that function called. If you really need that (never heard of someone who does), clear the frame's "_convert_image_callbacks" property and use `mlt_frame_append_convert_image()` instead. + - `fx_cut` (Shotcut Adjustment clip) now applies its filters only to tracks + below it. Higher tracks composite on top of the filtered result. + New image-stack callbacks used by tractor stacking and this path: + - `mlt_frame_get_image_from_service()` + - `mlt_frame_get_image_with_fx_cut()` Modules - Image converters (`movit.convert`, `avcolor_space`, `imageconvert`) are diff --git a/src/framework/mlt.vers b/src/framework/mlt.vers index 781e7a8f7..14ad46aae 100644 --- a/src/framework/mlt.vers +++ b/src/framework/mlt.vers @@ -701,4 +701,6 @@ MLT_7.40.0 { MLT_7.42.0 { global: mlt_audio_format_id; + mlt_frame_get_image_from_service; + mlt_frame_get_image_with_fx_cut; } MLT_7.40.0; diff --git a/src/framework/mlt_frame.c b/src/framework/mlt_frame.c index 86d70bd7c..3c68e23a6 100644 --- a/src/framework/mlt_frame.c +++ b/src/framework/mlt_frame.c @@ -1030,6 +1030,157 @@ void mlt_frame_copy_convert_image(mlt_frame dst, mlt_frame src) NULL); } +/** Copy consumer scaling hints from \p src onto \p dst. */ +static void copy_consumer_image_hints(mlt_frame dst, mlt_frame src) +{ + mlt_properties dst_properties = MLT_FRAME_PROPERTIES(dst); + mlt_properties src_properties = MLT_FRAME_PROPERTIES(src); + + mlt_properties_set_int(dst_properties, + "resize_alpha", + mlt_properties_get_int(src_properties, "resize_alpha")); + mlt_properties_set_int(dst_properties, + "distort", + mlt_properties_get_int(src_properties, "distort")); + mlt_properties_copy(dst_properties, src_properties, "consumer."); + // WebVfx uses this to setup a consumer-stopping event handler. + mlt_properties_set_data(dst_properties, + "consumer", + mlt_properties_get_data(src_properties, "consumer", NULL), + 0, + NULL, + NULL); +} + +/** Copy image properties, alpha, converters, and Movit state from \p src to \p dst. + * + * Does not take ownership of the image or alpha buffers. + */ +static void copy_image_state(mlt_frame dst, mlt_frame src) +{ + mlt_properties dst_properties = MLT_FRAME_PROPERTIES(dst); + mlt_properties src_properties = MLT_FRAME_PROPERTIES(src); + int size = 0; + uint8_t *data; + + mlt_properties_set_int(dst_properties, "width", mlt_properties_get_int(src_properties, "width")); + mlt_properties_set_int(dst_properties, + "height", + mlt_properties_get_int(src_properties, "height")); + mlt_properties_set_int(dst_properties, + "format", + mlt_properties_get_int(src_properties, "format")); + mlt_properties_set_double(dst_properties, "aspect_ratio", mlt_frame_get_aspect_ratio(src)); + mlt_properties_pass_list( + dst_properties, + src_properties, + "progressive,distort,colorspace,full_range,force_full_luma,top_field_first,color_trc"); + + mlt_properties_set_data(dst_properties, + "movit.convert.fence", + mlt_properties_get_data(src_properties, "movit.convert.fence", NULL), + 0, + NULL, + NULL); + mlt_properties_set_data(dst_properties, + "movit.convert.texture", + mlt_properties_get_data(src_properties, "movit.convert.texture", NULL), + 0, + NULL, + NULL); + mlt_properties_set_int(dst_properties, + "movit.convert.use_texture", + mlt_properties_get_int(src_properties, "movit.convert.use_texture")); + int i; + for (i = 0; i < mlt_properties_count(src_properties); i++) { + char *name = mlt_properties_get_name(src_properties, i); + if (name && !strncmp(name, "_movit ", 7)) { + mlt_properties_set_data(dst_properties, + name, + mlt_properties_get_data_at(src_properties, i, NULL), + 0, + NULL, + NULL); + } + } + + data = mlt_frame_get_alpha_size(src, &size); + if (data) + mlt_frame_set_alpha(dst, data, size, NULL); + dst->convert_audio = src->convert_audio; + mlt_frame_copy_convert_image(dst, src); +} + +/** Share \p src's image buffer and image state onto \p dst without taking ownership. + * + * Both frames must remain alive while \p dst's image is used (typically both are + * stored on the tractor output frame). + */ +static void share_image(mlt_frame dst, mlt_frame src) +{ + int size = 0; + uint8_t *image = mlt_properties_get_data(MLT_FRAME_PROPERTIES(src), "image", &size); + mlt_frame_set_image(dst, image, 0, NULL); + copy_image_state(dst, src); +} + +/** Get an image from a source frame previously pushed with mlt_frame_push_service(). + * + * Used as an image-stack callback (tractor track stacking and fx_cut routing). + * Copies consumer scaling onto the source, fetches its image, then shares that + * buffer onto \p self without taking ownership, along with format, alpha, + * converters, and Movit state. + * + * \public \memberof mlt_frame_s + * \return true if the stacked source frame is missing + */ +int mlt_frame_get_image_from_service(mlt_frame self, + uint8_t **buffer, + mlt_image_format *format, + int *width, + int *height, + int writable) +{ + mlt_frame frame = mlt_frame_pop_service(self); + if (!frame) + return 1; + + copy_consumer_image_hints(frame, self); + mlt_frame_get_image(frame, buffer, format, width, height, writable); + share_image(self, frame); + return 0; +} + +/** Route A through an fx_cut frame's filter chain without compositing the dummy. + * + * Pops the fx_cut frame, feeds A's image through that frame's filters via the + * tractor stacking callback, then shares the filtered result back onto A. + * + * \public \memberof mlt_frame_s + */ +int mlt_frame_get_image_with_fx_cut(mlt_frame a_frame, + uint8_t **image, + mlt_image_format *format, + int *width, + int *height, + int writable) +{ + mlt_frame fx_frame = mlt_frame_pop_service(a_frame); + if (!fx_frame) + return mlt_frame_get_image(a_frame, image, format, width, height, writable); + + copy_consumer_image_hints(fx_frame, a_frame); + mlt_frame_copy_convert_image(fx_frame, a_frame); + + mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(fx_frame), mlt_frame_get_image_from_service); + mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(fx_frame), a_frame); + + int error = mlt_frame_get_image(fx_frame, image, format, width, height, writable); + if (!error) + share_image(a_frame, fx_frame); + return error; +} + /***** convenience functions *****/ void mlt_frame_write_ppm(mlt_frame frame) diff --git a/src/framework/mlt_frame.h b/src/framework/mlt_frame.h index e6cb9d25f..d1f34e2bf 100644 --- a/src/framework/mlt_frame.h +++ b/src/framework/mlt_frame.h @@ -195,6 +195,18 @@ MLT_EXPORT int mlt_frame_next_convert_image(mlt_frame self, mlt_image_format *format, mlt_image_format output); MLT_EXPORT void mlt_frame_copy_convert_image(mlt_frame dst, mlt_frame src); +MLT_EXPORT int mlt_frame_get_image_from_service(mlt_frame self, + uint8_t **buffer, + mlt_image_format *format, + int *width, + int *height, + int writable); +MLT_EXPORT int mlt_frame_get_image_with_fx_cut(mlt_frame self, + uint8_t **buffer, + mlt_image_format *format, + int *width, + int *height, + int writable); MLT_EXPORT mlt_frame mlt_frame_clone(mlt_frame self, int is_deep); MLT_EXPORT mlt_frame mlt_frame_clone_audio(mlt_frame self, int is_deep); MLT_EXPORT mlt_frame mlt_frame_clone_image(mlt_frame self, int is_deep); diff --git a/src/framework/mlt_playlist.h b/src/framework/mlt_playlist.h index bae7363d9..5b6ff8bb9 100644 --- a/src/framework/mlt_playlist.h +++ b/src/framework/mlt_playlist.h @@ -60,6 +60,7 @@ typedef struct playlist_entry_s playlist_entry; * automatically close producers as they are finished being used to free resources. * \properties \em meta.fx_cut Set true on a producer to indicate that it is a "fx_cut," * which is a way to add filters as a playlist entry - useful only in a multitrack. See FxCut in the docs. + * Filters apply to the composite of lower-index tracks; higher tracks composite on top. * \properties \em mix_in * \properties \em mix_out * \properties \em hide Set to 1 to hide the video (make it an audio-only track), diff --git a/src/framework/mlt_tractor.c b/src/framework/mlt_tractor.c index 19f928bdc..8d5d6970f 100644 --- a/src/framework/mlt_tractor.c +++ b/src/framework/mlt_tractor.c @@ -345,84 +345,6 @@ mlt_producer mlt_tractor_get_track(mlt_tractor self, int index) return mlt_multitrack_track(mlt_tractor_multitrack(self), index); } -static int producer_get_image(mlt_frame self, - uint8_t **buffer, - mlt_image_format *format, - int *width, - int *height, - int writable) -{ - uint8_t *data = NULL; - int size = 0; - mlt_properties properties = MLT_FRAME_PROPERTIES(self); - mlt_frame frame = mlt_frame_pop_service(self); - mlt_properties frame_properties = MLT_FRAME_PROPERTIES(frame); - - mlt_properties_set_int(frame_properties, - "resize_alpha", - mlt_properties_get_int(properties, "resize_alpha")); - mlt_properties_set_int(frame_properties, - "distort", - mlt_properties_get_int(properties, "distort")); - mlt_properties_copy(frame_properties, properties, "consumer."); - // WebVfx uses this to setup a consumer-stopping event handler. - mlt_properties_set_data(frame_properties, - "consumer", - mlt_properties_get_data(properties, "consumer", NULL), - 0, - NULL, - NULL); - - mlt_frame_get_image(frame, buffer, format, width, height, writable); - mlt_frame_set_image(self, *buffer, 0, NULL); - - mlt_properties_set_int(properties, "width", *width); - mlt_properties_set_int(properties, "height", *height); - mlt_properties_set_int(properties, "format", *format); - mlt_properties_set_double(properties, "aspect_ratio", mlt_frame_get_aspect_ratio(frame)); - // Pass all required frame properties - mlt_properties_pass_list( - properties, - frame_properties, - "progressive,distort,colorspace,full_range,force_full_luma,top_field_first,color_trc"); - - mlt_properties_set_data(properties, - "movit.convert.fence", - mlt_properties_get_data(frame_properties, "movit.convert.fence", NULL), - 0, - NULL, - NULL); - mlt_properties_set_data(properties, - "movit.convert.texture", - mlt_properties_get_data(frame_properties, "movit.convert.texture", NULL), - 0, - NULL, - NULL); - mlt_properties_set_int(properties, - "movit.convert.use_texture", - mlt_properties_get_int(frame_properties, "movit.convert.use_texture")); - int i; - for (i = 0; i < mlt_properties_count(frame_properties); i++) { - char *name = mlt_properties_get_name(frame_properties, i); - if (name && !strncmp(name, "_movit ", 7)) { - mlt_properties_set_data(properties, - name, - mlt_properties_get_data_at(frame_properties, i, NULL), - 0, - NULL, - NULL); - } - } - - data = mlt_frame_get_alpha_size(frame, &size); - if (data) { - mlt_frame_set_alpha(self, data, size, NULL); - } - self->convert_audio = frame->convert_audio; - mlt_frame_copy_convert_image(self, frame); - return 0; -} - static int producer_get_audio(mlt_frame self, void **buffer, mlt_audio_format *format, @@ -549,9 +471,12 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac // Check for last track done = mlt_properties_get_int(temp_properties, "last_track"); - // Handle fx only tracks + // Handle fx only tracks. Always hide video so the tractor does + // not apply fx_cut filters to the already-composited mix of all + // tracks; transitions wrap lower tracks instead. Audio hide is + // unchanged: mute the dummy only when no audio has been found yet. if (mlt_properties_get_int(temp_properties, "fx_cut")) { - int hide = (video == NULL ? 1 : 0) | (audio == NULL ? 2 : 0); + int hide = 1 | (audio == NULL ? 2 : 0); mlt_properties_set_int(temp_properties, "hide", hide); } @@ -577,7 +502,8 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac if (!done && !mlt_frame_is_test_card(temp) && !(mlt_properties_get_int(temp_properties, "hide") & 1)) { if (video != NULL) { - mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(temp), producer_get_image); + mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(temp), + mlt_frame_get_image_from_service); mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(temp), video); } video = temp; @@ -598,7 +524,7 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac if (video != NULL) { mlt_properties video_properties = MLT_FRAME_PROPERTIES(first_video); mlt_frame_push_service(*frame, video); - mlt_frame_push_service(*frame, producer_get_image); + mlt_frame_push_service(*frame, mlt_frame_get_image_from_service); mlt_properties_set_int(frame_properties, "width", mlt_properties_get_int(video_properties, "width")); diff --git a/src/framework/mlt_transition.c b/src/framework/mlt_transition.c index c40b3591c..728706253 100644 --- a/src/framework/mlt_transition.c +++ b/src/framework/mlt_transition.c @@ -380,6 +380,55 @@ static int get_image_b(mlt_frame b_frame, return mlt_frame_get_image(b_frame, image, format, width, height, writable); } +static void process_transition_pair(mlt_transition self, + mlt_frame_ptr frame, + mlt_frame a_frame_ptr, + mlt_frame b_frame_ptr, + int type, + int active, + int (*invalid)(mlt_frame)) +{ + if (!a_frame_ptr || !MLT_FRAME_PROPERTIES(a_frame_ptr)->local || !b_frame_ptr + || !MLT_FRAME_PROPERTIES(b_frame_ptr)->local) + return; + + int a_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide"); + int b_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide"); + int b_is_fx_cut = type == 1 + && mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "fx_cut"); + + if (b_is_fx_cut && !mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "fx_cut") + && !invalid(a_frame_ptr) && !(a_hide & type)) { + // Apply fx_cut filters to A; do not composite the dummy B. + mlt_frame_push_service(a_frame_ptr, b_frame_ptr); + mlt_frame_push_get_image(a_frame_ptr, mlt_frame_get_image_with_fx_cut); + mlt_properties_set_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide", b_hide | 1); + return; + } + + if (!active || (a_hide & type) || (b_hide & type)) + return; + + // Add hooks for pre-processing frames + mlt_frame_push_service(a_frame_ptr, self); + mlt_frame_push_get_image(a_frame_ptr, get_image_a); + mlt_frame_push_frame(b_frame_ptr, a_frame_ptr); + mlt_frame_push_service(b_frame_ptr, self); + mlt_frame_push_get_image(b_frame_ptr, get_image_b); + + // Process the transition + *frame = mlt_transition_process(self, a_frame_ptr, b_frame_ptr); + + // We need to ensure that the tractor doesn't consider this frame for output + if (*frame == a_frame_ptr) + b_hide |= type; + else + a_hide |= type; + + mlt_properties_set_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide", a_hide); + mlt_properties_set_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide", b_hide); +} + /** Get a frame from a transition. The logic is complex here. A transition is typically applied to frames on the a and @@ -484,7 +533,7 @@ static int transition_get_frame(mlt_service service, mlt_frame_ptr frame, int in } // Determine if we're active now - // fx_cut frames are processed by the tractor, not transitions + // fx_cut is not composited as B; it is applied as a filter wrap below mlt_properties b_props = MLT_FRAME_PROPERTIES(self->frames[b_frame]); active = a_frame != b_frame && !invalid(self->frames[b_frame]) && !mlt_properties_get_int(b_props, "fx_cut"); @@ -506,36 +555,13 @@ static int transition_get_frame(mlt_service service, mlt_frame_ptr frame, int in } // Finally, process the a and b frames - if (active && !mlt_properties_get_int(MLT_TRANSITION_PROPERTIES(self), "disable")) { + if (!mlt_properties_get_int(MLT_TRANSITION_PROPERTIES(self), "disable") + && a_frame <= b_track) { int frame_nb = (!reverse_order && a_frame <= b_track) ? a_frame : b_frame; mlt_frame a_frame_ptr = self->frames[frame_nb]; frame_nb = (!reverse_order || a_frame > b_track) ? b_frame : a_frame; mlt_frame b_frame_ptr = self->frames[frame_nb]; - if (a_frame_ptr && MLT_FRAME_PROPERTIES(a_frame_ptr)->local && b_frame_ptr - && MLT_FRAME_PROPERTIES(b_frame_ptr)->local) { - int a_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide"); - int b_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide"); - if (!(a_hide & type) && !(b_hide & type)) { - // Add hooks for pre-processing frames - mlt_frame_push_service(a_frame_ptr, self); - mlt_frame_push_get_image(a_frame_ptr, get_image_a); - mlt_frame_push_frame(b_frame_ptr, a_frame_ptr); - mlt_frame_push_service(b_frame_ptr, self); - mlt_frame_push_get_image(b_frame_ptr, get_image_b); - - // Process the transition - *frame = mlt_transition_process(self, a_frame_ptr, b_frame_ptr); - - // We need to ensure that the tractor doesn't consider this frame for output - if (*frame == a_frame_ptr) - b_hide |= type; - else - a_hide |= type; - - mlt_properties_set_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide", a_hide); - mlt_properties_set_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide", b_hide); - } - } + process_transition_pair(self, frame, a_frame_ptr, b_frame_ptr, type, active, invalid); } } diff --git a/src/tests/test_tractor/test_tractor.cpp b/src/tests/test_tractor/test_tractor.cpp index d53f4eda5..c89a13a79 100644 --- a/src/tests/test_tractor/test_tractor.cpp +++ b/src/tests/test_tractor/test_tractor.cpp @@ -23,6 +23,56 @@ #include using namespace Mlt; +static Producer makeColor(Profile &profile, const char *color) +{ + Producer p(profile, "color", color); + p.set("length", 10); + p.set_in_and_out(0, 9); + return p; +} + +static Producer makeFxCut(Profile &profile, Filter &filter) +{ + Producer fx = makeColor(profile, "0x00000000"); + fx.set("mlt_image_format", "rgba"); + fx.set("meta.fx_cut", 1); + fx.attach(filter); + return fx; +} + +static bool sampleCenterRgb(Frame *frame, int &r, int &g, int &b) +{ + mlt_image_format fmt = mlt_image_rgb; + int w = 0, h = 0; + uint8_t *image = frame->get_image(fmt, w, h, 0); + if (!image || w < 2 || h < 1) + return false; + int x = (w / 2) & ~1; + int y = h / 2; + if (fmt == mlt_image_rgb) { + int i = (y * w + x) * 3; + r = image[i]; + g = image[i + 1]; + b = image[i + 2]; + } else if (fmt == mlt_image_rgba) { + int i = (y * w + x) * 4; + r = image[i]; + g = image[i + 1]; + b = image[i + 2]; + } else if (fmt == mlt_image_yuv422) { + int off = y * w * 2 + x * 2; + int yy = image[off]; + int u = image[off + 1]; + int v = image[off + 3]; + r = qBound(0, (int) (1.164 * (yy - 16) + 1.596 * (v - 128)), 255); + g = qBound(0, (int) (1.164 * (yy - 16) - 0.813 * (v - 128) - 0.391 * (u - 128)), 255); + b = qBound(0, (int) (1.164 * (yy - 16) + 2.018 * (u - 128)), 255); + } else { + return false; + } + return true; +} + class TestTractor : public QObject { Q_OBJECT @@ -406,8 +456,8 @@ private Q_SLOTS: void ConvertImagePropagatesThroughMultitrack() { - // The tractor's producer_get_image calls mlt_frame_copy_convert_image to - // propagate converters from each track frame onto the merged frame. + // The tractor's mlt_frame_get_image_from_service calls mlt_frame_copy_convert_image + // to propagate converters from each track frame onto the merged frame. Tractor t(profile); QVERIFY(t.is_valid()); @@ -421,9 +471,9 @@ private Q_SLOTS: mlt_service_get_frame(MLT_PRODUCER_SERVICE(t.get_producer()), &merged, 0); QVERIFY(merged != NULL); - // Calling get_image triggers producer_get_image in the tractor, which pulls - // track frames through the loader filter chain (causing converters to be pushed - // onto them), then copies them onto the merged frame via + // Calling get_image triggers mlt_frame_get_image_from_service in the tractor, + // which pulls track frames through the loader filter chain (causing converters + // to be pushed onto them), then copies them onto the merged frame via // mlt_frame_copy_convert_image. uint8_t *image = NULL; mlt_image_format fmt = mlt_image_rgb; @@ -434,6 +484,110 @@ private Q_SLOTS: mlt_frame_close(merged); } + + void FxCutAppliesToTrackBelow() + { + Transition blend(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blend.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + track0.append(red); + track1.append(fx); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.plant_transition(blend, 0, 1); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + // brightness level=0 turns the lower red clip black. + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g < 40, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + + void FxCutDoesNotAffectTrackAbove() + { + Transition blendFx(profile, "composite"); + Transition blendOverlay(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blendFx.is_valid() || !blendOverlay.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + Producer green = makeColor(profile, "0x00ff00ff"); + QVERIFY(green.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + Playlist track2(profile); + track0.append(red); + track1.append(fx); + track2.append(green); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.set_track(track2, 2); + // Same a_track layout as Shotcut: both overlays blend onto the bottom video. + t.plant_transition(blendFx, 0, 1); + t.plant_transition(blendOverlay, 0, 2); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + // Opaque green overlay must stay green (not darkened by the fx_cut). + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g > 200, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + + void FxCutAloneDoesNotCrash() + { + Transition blend(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blend.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + + Playlist track0(profile); + track0.append(fx); + + Tractor t(profile); + t.set_track(track0, 0); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + mlt_image_format fmt = mlt_image_rgb; + int w = 0, h = 0; + uint8_t *image = frame->get_image(fmt, w, h, 0); + QVERIFY(image != NULL); + delete frame; + } }; QTEST_APPLESS_MAIN(TestTractor) From a32ab05806f2913ee01f16d25497f019019ad484 Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 16:28:35 -0700 Subject: [PATCH 2/6] Address review comments --- NEWS | 8 +- src/framework/mlt.vers | 4 +- src/framework/mlt_frame.c | 69 ++++++++++---- src/framework/mlt_frame.h | 14 +-- src/framework/mlt_playlist.h | 2 +- src/framework/mlt_tractor.c | 26 +++--- src/framework/mlt_transition.c | 29 ++++-- src/tests/test_tractor/test_tractor.cpp | 118 +++++++++++++++++++++++- 8 files changed, 206 insertions(+), 64 deletions(-) diff --git a/NEWS b/NEWS index 402ce2f3f..d3b3fc206 100644 --- a/NEWS +++ b/NEWS @@ -20,11 +20,11 @@ Framework longer have that function called. If you really need that (never heard of someone who does), clear the frame's "_convert_image_callbacks" property and use `mlt_frame_append_convert_image()` instead. - - `fx_cut` (Shotcut Adjustment clip) now applies its filters only to tracks + - `fx_cut` (Adjustment Clip) now applies its filters only to tracks below it. Higher tracks composite on top of the filtered result. - New image-stack callbacks used by tractor stacking and this path: - - `mlt_frame_get_image_from_service()` - - `mlt_frame_get_image_with_fx_cut()` + New image-stack helpers used by tractor stacking and this path: + - `mlt_frame_prepend_image_from_service()` + - `mlt_frame_push_image_with_fx_cut()` Modules - Image converters (`movit.convert`, `avcolor_space`, `imageconvert`) are diff --git a/src/framework/mlt.vers b/src/framework/mlt.vers index 14ad46aae..61e4c054d 100644 --- a/src/framework/mlt.vers +++ b/src/framework/mlt.vers @@ -701,6 +701,6 @@ MLT_7.40.0 { MLT_7.42.0 { global: mlt_audio_format_id; - mlt_frame_get_image_from_service; - mlt_frame_get_image_with_fx_cut; + mlt_frame_prepend_image_from_service; + mlt_frame_push_image_with_fx_cut; } MLT_7.40.0; diff --git a/src/framework/mlt_frame.c b/src/framework/mlt_frame.c index 3c68e23a6..26da744de 100644 --- a/src/framework/mlt_frame.c +++ b/src/framework/mlt_frame.c @@ -1124,22 +1124,22 @@ static void share_image(mlt_frame dst, mlt_frame src) copy_image_state(dst, src); } -/** Get an image from a source frame previously pushed with mlt_frame_push_service(). +/** Get an image from a source frame previously installed by + * mlt_frame_prepend_image_from_service(). * - * Used as an image-stack callback (tractor track stacking and fx_cut routing). * Copies consumer scaling onto the source, fetches its image, then shares that * buffer onto \p self without taking ownership, along with format, alpha, * converters, and Movit state. * - * \public \memberof mlt_frame_s + * \private \memberof mlt_frame_s * \return true if the stacked source frame is missing */ -int mlt_frame_get_image_from_service(mlt_frame self, - uint8_t **buffer, - mlt_image_format *format, - int *width, - int *height, - int writable) +static int get_image_from_service(mlt_frame self, + uint8_t **buffer, + mlt_image_format *format, + int *width, + int *height, + int writable) { mlt_frame frame = mlt_frame_pop_service(self); if (!frame) @@ -1151,19 +1151,36 @@ int mlt_frame_get_image_from_service(mlt_frame self, return 0; } -/** Route A through an fx_cut frame's filter chain without compositing the dummy. +/** Install \p source as the image under this frame's existing get_image callbacks. * - * Pops the fx_cut frame, feeds A's image through that frame's filters via the - * tractor stacking callback, then shares the filtered result back onto A. + * Prepends so filters already on \p self run first and read \p source. On an + * empty stack this is equivalent to pushing the source then the callback. + * Used by tractor track stacking and the tractor output frame. * * \public \memberof mlt_frame_s + * \return true if error */ -int mlt_frame_get_image_with_fx_cut(mlt_frame a_frame, - uint8_t **image, - mlt_image_format *format, - int *width, - int *height, - int writable) +int mlt_frame_prepend_image_from_service(mlt_frame self, mlt_frame source) +{ + int error = mlt_deque_push_front(self->stack_image, get_image_from_service); + if (!error) + error = mlt_deque_push_front(self->stack_image, source); + return error; +} + +/** Route \p self through \p fx's filter chain without compositing the dummy. + * + * Pops the fx_cut frame, feeds this frame's image through that frame's filters + * via mlt_frame_prepend_image_from_service(), then shares the filtered result back. + * + * \private \memberof mlt_frame_s + */ +static int get_image_with_fx_cut(mlt_frame a_frame, + uint8_t **image, + mlt_image_format *format, + int *width, + int *height, + int writable) { mlt_frame fx_frame = mlt_frame_pop_service(a_frame); if (!fx_frame) @@ -1172,8 +1189,7 @@ int mlt_frame_get_image_with_fx_cut(mlt_frame a_frame, copy_consumer_image_hints(fx_frame, a_frame); mlt_frame_copy_convert_image(fx_frame, a_frame); - mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(fx_frame), mlt_frame_get_image_from_service); - mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(fx_frame), a_frame); + mlt_frame_prepend_image_from_service(fx_frame, a_frame); int error = mlt_frame_get_image(fx_frame, image, format, width, height, writable); if (!error) @@ -1181,6 +1197,19 @@ int mlt_frame_get_image_with_fx_cut(mlt_frame a_frame, return error; } +/** Push an fx_cut wrap so \p self is filtered through \p fx on get_image. + * + * \public \memberof mlt_frame_s + * \return true if error + */ +int mlt_frame_push_image_with_fx_cut(mlt_frame self, mlt_frame fx) +{ + int error = mlt_frame_push_service(self, fx); + if (!error) + error = mlt_frame_push_get_image(self, get_image_with_fx_cut); + return error; +} + /***** convenience functions *****/ void mlt_frame_write_ppm(mlt_frame frame) diff --git a/src/framework/mlt_frame.h b/src/framework/mlt_frame.h index d1f34e2bf..9958b5540 100644 --- a/src/framework/mlt_frame.h +++ b/src/framework/mlt_frame.h @@ -178,6 +178,8 @@ MLT_EXPORT int mlt_frame_push_service_int(mlt_frame self, int that); MLT_EXPORT int mlt_frame_pop_service_int(mlt_frame self); MLT_EXPORT int mlt_frame_push_audio(mlt_frame self, void *that); MLT_EXPORT void *mlt_frame_pop_audio(mlt_frame self); +MLT_EXPORT int mlt_frame_prepend_image_from_service(mlt_frame self, mlt_frame source); +MLT_EXPORT int mlt_frame_push_image_with_fx_cut(mlt_frame self, mlt_frame fx); MLT_EXPORT mlt_deque mlt_frame_service_stack(mlt_frame self); MLT_EXPORT mlt_producer mlt_frame_get_original_producer(mlt_frame self); MLT_EXPORT void mlt_frame_close(mlt_frame self); @@ -195,18 +197,6 @@ MLT_EXPORT int mlt_frame_next_convert_image(mlt_frame self, mlt_image_format *format, mlt_image_format output); MLT_EXPORT void mlt_frame_copy_convert_image(mlt_frame dst, mlt_frame src); -MLT_EXPORT int mlt_frame_get_image_from_service(mlt_frame self, - uint8_t **buffer, - mlt_image_format *format, - int *width, - int *height, - int writable); -MLT_EXPORT int mlt_frame_get_image_with_fx_cut(mlt_frame self, - uint8_t **buffer, - mlt_image_format *format, - int *width, - int *height, - int writable); MLT_EXPORT mlt_frame mlt_frame_clone(mlt_frame self, int is_deep); MLT_EXPORT mlt_frame mlt_frame_clone_audio(mlt_frame self, int is_deep); MLT_EXPORT mlt_frame mlt_frame_clone_image(mlt_frame self, int is_deep); diff --git a/src/framework/mlt_playlist.h b/src/framework/mlt_playlist.h index 5b6ff8bb9..cfe7f8625 100644 --- a/src/framework/mlt_playlist.h +++ b/src/framework/mlt_playlist.h @@ -3,7 +3,7 @@ * \brief playlist service class * \see mlt_playlist_s * - * Copyright (C) 2003-2022 Meltytech, LLC + * Copyright (C) 2003-2026 Meltytech, LLC * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public diff --git a/src/framework/mlt_tractor.c b/src/framework/mlt_tractor.c index 8d5d6970f..66c95114a 100644 --- a/src/framework/mlt_tractor.c +++ b/src/framework/mlt_tractor.c @@ -471,12 +471,18 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac // Check for last track done = mlt_properties_get_int(temp_properties, "last_track"); - // Handle fx only tracks. Always hide video so the tractor does - // not apply fx_cut filters to the already-composited mix of all - // tracks; transitions wrap lower tracks instead. Audio hide is - // unchanged: mute the dummy only when no audio has been found yet. + // Handle fx only tracks. Hide the dummy when nothing below has + // supplied video/audio yet. If a transition already hid this + // frame, keep that hide bit — do not overwrite — so tractor + // stacking does not re-apply filters to the full composite. + // With no transition, leave video visible so the fx_cut can + // filter the lower track (classic melt path). if (mlt_properties_get_int(temp_properties, "fx_cut")) { - int hide = 1 | (audio == NULL ? 2 : 0); + int hide = mlt_properties_get_int(temp_properties, "hide"); + if (video == NULL) + hide |= 1; + if (audio == NULL) + hide |= 2; mlt_properties_set_int(temp_properties, "hide", hide); } @@ -501,11 +507,8 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac } if (!done && !mlt_frame_is_test_card(temp) && !(mlt_properties_get_int(temp_properties, "hide") & 1)) { - if (video != NULL) { - mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(temp), - mlt_frame_get_image_from_service); - mlt_deque_push_front(MLT_FRAME_IMAGE_STACK(temp), video); - } + if (video != NULL) + mlt_frame_prepend_image_from_service(temp, video); video = temp; if (first_video == NULL) first_video = temp; @@ -523,8 +526,7 @@ static int producer_get_frame(mlt_producer parent, mlt_frame_ptr frame, int trac if (video != NULL) { mlt_properties video_properties = MLT_FRAME_PROPERTIES(first_video); - mlt_frame_push_service(*frame, video); - mlt_frame_push_service(*frame, mlt_frame_get_image_from_service); + mlt_frame_prepend_image_from_service(*frame, video); mlt_properties_set_int(frame_properties, "width", mlt_properties_get_int(video_properties, "width")); diff --git a/src/framework/mlt_transition.c b/src/framework/mlt_transition.c index 728706253..d7e13b364 100644 --- a/src/framework/mlt_transition.c +++ b/src/framework/mlt_transition.c @@ -380,6 +380,11 @@ static int get_image_b(mlt_frame b_frame, return mlt_frame_get_image(b_frame, image, format, width, height, writable); } +static int frame_is_fx_cut(mlt_frame frame, int type) +{ + return type == 1 && mlt_properties_get_int(MLT_FRAME_PROPERTIES(frame), "fx_cut"); +} + static void process_transition_pair(mlt_transition self, mlt_frame_ptr frame, mlt_frame a_frame_ptr, @@ -394,15 +399,19 @@ static void process_transition_pair(mlt_transition self, int a_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide"); int b_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide"); - int b_is_fx_cut = type == 1 - && mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "fx_cut"); - - if (b_is_fx_cut && !mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "fx_cut") - && !invalid(a_frame_ptr) && !(a_hide & type)) { - // Apply fx_cut filters to A; do not composite the dummy B. - mlt_frame_push_service(a_frame_ptr, b_frame_ptr); - mlt_frame_push_get_image(a_frame_ptr, mlt_frame_get_image_with_fx_cut); - mlt_properties_set_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide", b_hide | 1); + + // Wrap is not a composite: fx may be A or B after reverse_order mapping. + mlt_frame picture = a_frame_ptr; + mlt_frame fx = b_frame_ptr; + if (frame_is_fx_cut(a_frame_ptr, type) && !frame_is_fx_cut(b_frame_ptr, type)) { + picture = b_frame_ptr; + fx = a_frame_ptr; + } + if (frame_is_fx_cut(fx, type) && !frame_is_fx_cut(picture, type) && !invalid(picture) + && !(mlt_properties_get_int(MLT_FRAME_PROPERTIES(picture), "hide") & type)) { + mlt_frame_push_image_with_fx_cut(picture, fx); + int fx_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(fx), "hide"); + mlt_properties_set_int(MLT_FRAME_PROPERTIES(fx), "hide", fx_hide | 1); return; } @@ -533,7 +542,7 @@ static int transition_get_frame(mlt_service service, mlt_frame_ptr frame, int in } // Determine if we're active now - // fx_cut is not composited as B; it is applied as a filter wrap below + // fx_cut is not composited; process_transition_pair wraps it mlt_properties b_props = MLT_FRAME_PROPERTIES(self->frames[b_frame]); active = a_frame != b_frame && !invalid(self->frames[b_frame]) && !mlt_properties_get_int(b_props, "fx_cut"); diff --git a/src/tests/test_tractor/test_tractor.cpp b/src/tests/test_tractor/test_tractor.cpp index c89a13a79..e964e254a 100644 --- a/src/tests/test_tractor/test_tractor.cpp +++ b/src/tests/test_tractor/test_tractor.cpp @@ -456,8 +456,9 @@ private Q_SLOTS: void ConvertImagePropagatesThroughMultitrack() { - // The tractor's mlt_frame_get_image_from_service calls mlt_frame_copy_convert_image - // to propagate converters from each track frame onto the merged frame. + // The tractor's mlt_frame_prepend_image_from_service path calls + // mlt_frame_copy_convert_image to propagate converters from each track + // frame onto the merged frame. Tractor t(profile); QVERIFY(t.is_valid()); @@ -471,7 +472,7 @@ private Q_SLOTS: mlt_service_get_frame(MLT_PRODUCER_SERVICE(t.get_producer()), &merged, 0); QVERIFY(merged != NULL); - // Calling get_image triggers mlt_frame_get_image_from_service in the tractor, + // Calling get_image triggers the tractor's from-service image callback, // which pulls track frames through the loader filter chain (causing converters // to be pushed onto them), then copies them onto the merged frame via // mlt_frame_copy_convert_image. @@ -520,6 +521,74 @@ private Q_SLOTS: delete frame; } + void FxCutAppliesToTrackBelowWithReversedTracks() + { + Transition blend(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blend.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + track0.append(red); + track1.append(fx); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + // a_track > b_track: reverse_order must still wrap the lower clip. + t.plant_transition(blend, 1, 0); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g < 40, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + + void FxCutAppliesToTrackBelowWithoutTransition() + { + Filter brightness(profile, "brightness"); + if (!brightness.is_valid()) + QSKIP("brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + track0.append(red); + track1.append(fx); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + // Classic tractor stacking: fx_cut filters the lower track with no blend. + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g < 40, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + void FxCutDoesNotAffectTrackAbove() { Transition blendFx(profile, "composite"); @@ -563,6 +632,49 @@ private Q_SLOTS: delete frame; } + void FxCutDoesNotAffectTrackAboveWithReversedTracks() + { + Transition blendFx(profile, "composite"); + Transition blendOverlay(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blendFx.is_valid() || !blendOverlay.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + Producer green = makeColor(profile, "0x00ff00ff"); + QVERIFY(green.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + Playlist track2(profile); + track0.append(red); + track1.append(fx); + track2.append(green); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.set_track(track2, 2); + // Reversed fx blend: wrap must still hide the fx_cut so stacking does + // not grade the overlay. + t.plant_transition(blendFx, 1, 0); + t.plant_transition(blendOverlay, 0, 2); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g > 200, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + void FxCutAloneDoesNotCrash() { Transition blend(profile, "composite"); From 96379e900680bccaa25f666f9383ca862f94a128 Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 17:04:33 -0700 Subject: [PATCH 3/6] Implement fx_cut data sharing and processing in transitions --- src/framework/mlt_frame.c | 50 +++++++-------- src/framework/mlt_transition.c | 53 +++++++++++----- src/tests/test_tractor/test_tractor.cpp | 83 +++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 42 deletions(-) diff --git a/src/framework/mlt_frame.c b/src/framework/mlt_frame.c index 26da744de..1fd29a8d4 100644 --- a/src/framework/mlt_frame.c +++ b/src/framework/mlt_frame.c @@ -1052,6 +1052,18 @@ static void copy_consumer_image_hints(mlt_frame dst, mlt_frame src) NULL); } +/** Install \p data on \p dst without taking ownership. + * + * Skips the set when \p dst already holds \p data: mlt_property_set_data + * nulls the destructor if the pointer is unchanged, which would leak the + * original owner's buffer (fx_cut wrap shares onto the fx frame and back). + */ +static void share_data(mlt_properties dst, const char *name, void *data, int size) +{ + if (data && data != mlt_properties_get_data(dst, name, NULL)) + mlt_properties_set_data(dst, name, data, size, NULL, NULL); +} + /** Copy image properties, alpha, converters, and Movit state from \p src to \p dst. * * Does not take ownership of the image or alpha buffers. @@ -1076,37 +1088,26 @@ static void copy_image_state(mlt_frame dst, mlt_frame src) src_properties, "progressive,distort,colorspace,full_range,force_full_luma,top_field_first,color_trc"); - mlt_properties_set_data(dst_properties, - "movit.convert.fence", - mlt_properties_get_data(src_properties, "movit.convert.fence", NULL), - 0, - NULL, - NULL); - mlt_properties_set_data(dst_properties, - "movit.convert.texture", - mlt_properties_get_data(src_properties, "movit.convert.texture", NULL), - 0, - NULL, - NULL); + share_data(dst_properties, + "movit.convert.fence", + mlt_properties_get_data(src_properties, "movit.convert.fence", NULL), + 0); + share_data(dst_properties, + "movit.convert.texture", + mlt_properties_get_data(src_properties, "movit.convert.texture", NULL), + 0); mlt_properties_set_int(dst_properties, "movit.convert.use_texture", mlt_properties_get_int(src_properties, "movit.convert.use_texture")); int i; for (i = 0; i < mlt_properties_count(src_properties); i++) { char *name = mlt_properties_get_name(src_properties, i); - if (name && !strncmp(name, "_movit ", 7)) { - mlt_properties_set_data(dst_properties, - name, - mlt_properties_get_data_at(src_properties, i, NULL), - 0, - NULL, - NULL); - } + if (name && !strncmp(name, "_movit ", 7)) + share_data(dst_properties, name, mlt_properties_get_data_at(src_properties, i, NULL), 0); } data = mlt_frame_get_alpha_size(src, &size); - if (data) - mlt_frame_set_alpha(dst, data, size, NULL); + share_data(dst_properties, "alpha", data, size); dst->convert_audio = src->convert_audio; mlt_frame_copy_convert_image(dst, src); } @@ -1118,9 +1119,8 @@ static void copy_image_state(mlt_frame dst, mlt_frame src) */ static void share_image(mlt_frame dst, mlt_frame src) { - int size = 0; - uint8_t *image = mlt_properties_get_data(MLT_FRAME_PROPERTIES(src), "image", &size); - mlt_frame_set_image(dst, image, 0, NULL); + uint8_t *image = mlt_properties_get_data(MLT_FRAME_PROPERTIES(src), "image", NULL); + share_data(MLT_FRAME_PROPERTIES(dst), "image", image, 0); copy_image_state(dst, src); } diff --git a/src/framework/mlt_transition.c b/src/framework/mlt_transition.c index d7e13b364..78a88b428 100644 --- a/src/framework/mlt_transition.c +++ b/src/framework/mlt_transition.c @@ -385,13 +385,43 @@ static int frame_is_fx_cut(mlt_frame frame, int type) return type == 1 && mlt_properties_get_int(MLT_FRAME_PROPERTIES(frame), "fx_cut"); } +/** Route \p picture through \p fx and hide the dummy. No-op if already hidden. */ +static void wrap_fx_cut(mlt_frame picture, mlt_frame fx) +{ + int fx_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(fx), "hide"); + if (fx_hide & 1) + return; + mlt_frame_push_image_with_fx_cut(picture, fx); + mlt_properties_set_int(MLT_FRAME_PROPERTIES(fx), "hide", fx_hide | 1); +} + +/** Wrap every fx_cut from the hunted picture through B, inclusive. + * + * Physical indices, so reverse_order does not matter. Includes B so a 0→1 + * blend and a 0→2 blend with fx_cut on track 1 share this path. Hide on the + * dummy prevents a second wrap from an outer transition. + */ +static void apply_fx_cuts( + mlt_transition self, int a_frame, int b_frame, int type, int (*invalid)(mlt_frame)) +{ + mlt_frame picture = self->frames[a_frame]; + if (!picture || !MLT_FRAME_PROPERTIES(picture)->local || invalid(picture) + || frame_is_fx_cut(picture, type) + || (mlt_properties_get_int(MLT_FRAME_PROPERTIES(picture), "hide") & type)) + return; + for (int i = a_frame + 1; i <= b_frame; i++) { + mlt_frame fx = self->frames[i]; + if (fx && MLT_FRAME_PROPERTIES(fx)->local && frame_is_fx_cut(fx, type)) + wrap_fx_cut(picture, fx); + } +} + static void process_transition_pair(mlt_transition self, mlt_frame_ptr frame, mlt_frame a_frame_ptr, mlt_frame b_frame_ptr, int type, - int active, - int (*invalid)(mlt_frame)) + int active) { if (!a_frame_ptr || !MLT_FRAME_PROPERTIES(a_frame_ptr)->local || !b_frame_ptr || !MLT_FRAME_PROPERTIES(b_frame_ptr)->local) @@ -400,20 +430,8 @@ static void process_transition_pair(mlt_transition self, int a_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(a_frame_ptr), "hide"); int b_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(b_frame_ptr), "hide"); - // Wrap is not a composite: fx may be A or B after reverse_order mapping. - mlt_frame picture = a_frame_ptr; - mlt_frame fx = b_frame_ptr; - if (frame_is_fx_cut(a_frame_ptr, type) && !frame_is_fx_cut(b_frame_ptr, type)) { - picture = b_frame_ptr; - fx = a_frame_ptr; - } - if (frame_is_fx_cut(fx, type) && !frame_is_fx_cut(picture, type) && !invalid(picture) - && !(mlt_properties_get_int(MLT_FRAME_PROPERTIES(picture), "hide") & type)) { - mlt_frame_push_image_with_fx_cut(picture, fx); - int fx_hide = mlt_properties_get_int(MLT_FRAME_PROPERTIES(fx), "hide"); - mlt_properties_set_int(MLT_FRAME_PROPERTIES(fx), "hide", fx_hide | 1); + if (frame_is_fx_cut(a_frame_ptr, type) || frame_is_fx_cut(b_frame_ptr, type)) return; - } if (!active || (a_hide & type) || (b_hide & type)) return; @@ -542,7 +560,7 @@ static int transition_get_frame(mlt_service service, mlt_frame_ptr frame, int in } // Determine if we're active now - // fx_cut is not composited; process_transition_pair wraps it + // fx_cut is not composited; apply_fx_cuts wraps it mlt_properties b_props = MLT_FRAME_PROPERTIES(self->frames[b_frame]); active = a_frame != b_frame && !invalid(self->frames[b_frame]) && !mlt_properties_get_int(b_props, "fx_cut"); @@ -566,11 +584,12 @@ static int transition_get_frame(mlt_service service, mlt_frame_ptr frame, int in // Finally, process the a and b frames if (!mlt_properties_get_int(MLT_TRANSITION_PROPERTIES(self), "disable") && a_frame <= b_track) { + apply_fx_cuts(self, a_frame, b_frame, type, invalid); int frame_nb = (!reverse_order && a_frame <= b_track) ? a_frame : b_frame; mlt_frame a_frame_ptr = self->frames[frame_nb]; frame_nb = (!reverse_order || a_frame > b_track) ? b_frame : a_frame; mlt_frame b_frame_ptr = self->frames[frame_nb]; - process_transition_pair(self, frame, a_frame_ptr, b_frame_ptr, type, active, invalid); + process_transition_pair(self, frame, a_frame_ptr, b_frame_ptr, type, active); } } diff --git a/src/tests/test_tractor/test_tractor.cpp b/src/tests/test_tractor/test_tractor.cpp index e964e254a..9a0c04def 100644 --- a/src/tests/test_tractor/test_tractor.cpp +++ b/src/tests/test_tractor/test_tractor.cpp @@ -632,6 +632,89 @@ private Q_SLOTS: delete frame; } + void FxCutDoesNotAffectTrackAboveWithoutFxTransition() + { + Transition blendOverlay(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blendOverlay.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + Producer green = makeColor(profile, "0x00ff00ff"); + QVERIFY(green.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + Playlist track2(profile); + track0.append(red); + track1.append(fx); + track2.append(green); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.set_track(track2, 2); + // Only a 0→2 composite: wrap the in-between fx_cut before blending so + // tractor stacking cannot grade the overlay. + t.plant_transition(blendOverlay, 0, 2); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g > 200, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + + void FxCutDoesNotAffectTrackAboveWithDisabledFxTransition() + { + Transition blendFx(profile, "composite"); + Transition blendOverlay(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blendFx.is_valid() || !blendOverlay.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + blendFx.set("disable", 1); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + Producer green = makeColor(profile, "0x00ff00ff"); + QVERIFY(green.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + Playlist track2(profile); + track0.append(red); + track1.append(fx); + track2.append(green); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.set_track(track2, 2); + t.plant_transition(blendFx, 0, 1); + t.plant_transition(blendOverlay, 0, 2); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g > 200, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + void FxCutDoesNotAffectTrackAboveWithReversedTracks() { Transition blendFx(profile, "composite"); From 49483c9abe476003ffec812d566f9a523b9d1784 Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 17:12:43 -0700 Subject: [PATCH 4/6] Fix data sharing condition in share_data function Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/framework/mlt_frame.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/framework/mlt_frame.c b/src/framework/mlt_frame.c index 1fd29a8d4..a3443470d 100644 --- a/src/framework/mlt_frame.c +++ b/src/framework/mlt_frame.c @@ -1060,7 +1060,8 @@ static void copy_consumer_image_hints(mlt_frame dst, mlt_frame src) */ static void share_data(mlt_properties dst, const char *name, void *data, int size) { - if (data && data != mlt_properties_get_data(dst, name, NULL)) + void *current = mlt_properties_get_data(dst, name, NULL); + if (data != current) mlt_properties_set_data(dst, name, data, size, NULL, NULL); } From e77505bc76451a074af3e6ff3736cb5bd25db936 Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 17:26:19 -0700 Subject: [PATCH 5/6] Update share_image function to accept image pointer --- src/framework/mlt_frame.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/framework/mlt_frame.c b/src/framework/mlt_frame.c index a3443470d..90c481a18 100644 --- a/src/framework/mlt_frame.c +++ b/src/framework/mlt_frame.c @@ -1113,14 +1113,14 @@ static void copy_image_state(mlt_frame dst, mlt_frame src) mlt_frame_copy_convert_image(dst, src); } -/** Share \p src's image buffer and image state onto \p dst without taking ownership. +/** Share \p image and \p src's image state onto \p dst without taking ownership. * - * Both frames must remain alive while \p dst's image is used (typically both are - * stored on the tractor output frame). + * \p image is the pointer returned by mlt_frame_get_image(), which need not be + * stored in src's "image" property. Both frames must remain alive while \p dst's + * image is used (typically both are stored on the tractor output frame). */ -static void share_image(mlt_frame dst, mlt_frame src) +static void share_image(mlt_frame dst, mlt_frame src, uint8_t *image) { - uint8_t *image = mlt_properties_get_data(MLT_FRAME_PROPERTIES(src), "image", NULL); share_data(MLT_FRAME_PROPERTIES(dst), "image", image, 0); copy_image_state(dst, src); } @@ -1148,7 +1148,7 @@ static int get_image_from_service(mlt_frame self, copy_consumer_image_hints(frame, self); mlt_frame_get_image(frame, buffer, format, width, height, writable); - share_image(self, frame); + share_image(self, frame, (buffer && *buffer) ? *buffer : NULL); return 0; } @@ -1194,7 +1194,7 @@ static int get_image_with_fx_cut(mlt_frame a_frame, int error = mlt_frame_get_image(fx_frame, image, format, width, height, writable); if (!error) - share_image(a_frame, fx_frame); + share_image(a_frame, fx_frame, (image && *image) ? *image : NULL); return error; } From 49a2b08ec9c6873c264bbd4114307410706b11be Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Thu, 3 Sep 2026 17:44:11 -0700 Subject: [PATCH 6/6] add visibility checks and improve frame wrapping logic --- src/framework/mlt_transition.c | 31 ++++++++++++----- src/tests/test_tractor/test_tractor.cpp | 46 +++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/framework/mlt_transition.c b/src/framework/mlt_transition.c index 78a88b428..7c352a464 100644 --- a/src/framework/mlt_transition.c +++ b/src/framework/mlt_transition.c @@ -385,6 +385,13 @@ static int frame_is_fx_cut(mlt_frame frame, int type) return type == 1 && mlt_properties_get_int(MLT_FRAME_PROPERTIES(frame), "fx_cut"); } +static int frame_is_visible_picture(mlt_frame frame, int type, int (*invalid)(mlt_frame)) +{ + return frame && MLT_FRAME_PROPERTIES(frame)->local && !invalid(frame) + && !frame_is_fx_cut(frame, type) + && !(mlt_properties_get_int(MLT_FRAME_PROPERTIES(frame), "hide") & type); +} + /** Route \p picture through \p fx and hide the dummy. No-op if already hidden. */ static void wrap_fx_cut(mlt_frame picture, mlt_frame fx) { @@ -395,24 +402,30 @@ static void wrap_fx_cut(mlt_frame picture, mlt_frame fx) mlt_properties_set_int(MLT_FRAME_PROPERTIES(fx), "hide", fx_hide | 1); } -/** Wrap every fx_cut from the hunted picture through B, inclusive. +/** Wrap each fx_cut onto the accumulated visible result below it. * - * Physical indices, so reverse_order does not matter. Includes B so a 0→1 - * blend and a 0→2 blend with fx_cut on track 1 share this path. Hide on the + * Walk physical indices through B so reverse_order does not matter. A visible + * video between A and a later cut becomes the wrap target (and stacks the + * previous picture underneath) so the cut filters all lower-index tracks, not + * only hunted A. B itself is not accumulated — it is the overlay. Hide on the * dummy prevents a second wrap from an outer transition. */ static void apply_fx_cuts( mlt_transition self, int a_frame, int b_frame, int type, int (*invalid)(mlt_frame)) { mlt_frame picture = self->frames[a_frame]; - if (!picture || !MLT_FRAME_PROPERTIES(picture)->local || invalid(picture) - || frame_is_fx_cut(picture, type) - || (mlt_properties_get_int(MLT_FRAME_PROPERTIES(picture), "hide") & type)) + if (!frame_is_visible_picture(picture, type, invalid)) return; for (int i = a_frame + 1; i <= b_frame; i++) { - mlt_frame fx = self->frames[i]; - if (fx && MLT_FRAME_PROPERTIES(fx)->local && frame_is_fx_cut(fx, type)) - wrap_fx_cut(picture, fx); + mlt_frame f = self->frames[i]; + if (!f || !MLT_FRAME_PROPERTIES(f)->local) + continue; + if (frame_is_fx_cut(f, type)) + wrap_fx_cut(picture, f); + else if (i < b_frame && frame_is_visible_picture(f, type, invalid)) { + mlt_frame_prepend_image_from_service(f, picture); + picture = f; + } } } diff --git a/src/tests/test_tractor/test_tractor.cpp b/src/tests/test_tractor/test_tractor.cpp index 9a0c04def..c2ce925a7 100644 --- a/src/tests/test_tractor/test_tractor.cpp +++ b/src/tests/test_tractor/test_tractor.cpp @@ -632,6 +632,52 @@ private Q_SLOTS: delete frame; } + void FxCutAppliesToIntermediateTrackWithoutDirectTransition() + { + Transition blend(profile, "composite"); + Filter brightness(profile, "brightness"); + if (!blend.is_valid() || !brightness.is_valid()) + QSKIP("composite or brightness not available"); + brightness.set("level", 0.0); + + Producer red = makeColor(profile, "0xff0000ff"); + QVERIFY(red.is_valid()); + Producer green = makeColor(profile, "0x00ff00ff"); + QVERIFY(green.is_valid()); + Producer fx = makeFxCut(profile, brightness); + QVERIFY(fx.is_valid()); + Producer blue = makeColor(profile, "0x0000ffff"); + QVERIFY(blue.is_valid()); + + Playlist track0(profile); + Playlist track1(profile); + Playlist track2(profile); + Playlist track3(profile); + track0.append(red); + track1.append(green); + track2.append(fx); + track3.append(blue); + + Tractor t(profile); + t.set_track(track0, 0); + t.set_track(track1, 1); + t.set_track(track2, 2); + t.set_track(track3, 3); + // Only 0→3: the cut must wrap the nearest visible lower track (green), + // not only hunted A, so tractor stacking cannot bypass it. + t.plant_transition(blend, 0, 3); + + Frame *frame = t.get_frame(); + QVERIFY(frame != NULL); + int r = 0, g = 0, b = 0; + QVERIFY(sampleCenterRgb(frame, r, g, b)); + QString pixel = QString("rgb=%1,%2,%3").arg(r).arg(g).arg(b); + QVERIFY2(r < 40, qPrintable(pixel)); + QVERIFY2(g < 40, qPrintable(pixel)); + QVERIFY2(b < 40, qPrintable(pixel)); + delete frame; + } + void FxCutDoesNotAffectTrackAboveWithoutFxTransition() { Transition blendOverlay(profile, "composite");