Skip to content

Fix crash applying a Movit filter to an fx_cut - #1297

Merged
ddennedy merged 3 commits into
masterfrom
fix-fx-cut-movit
Sep 12, 2026
Merged

ddennedy merged 3 commits into
masterfrom
fix-fx-cut-movit

Conversation

@ddennedy

Copy link
Copy Markdown
Member

Playlist fx_cut frames skipped mlt_producer_get_frame, so _producer was unset and Movit keyed the chain with a NULL unique id. Pulling mlt_image_movit from another producer also shared an in-progress GPU chain onto the cut.

Finalize that chain to rgba64 before sharing, do not copy producer-keyed _movit state across the boundary, and upload rgba64 as GL_UNSIGNED_SHORT so 10-bit OpenGL filters on an Adjustment Clip are not distorted.

Playlist fx_cut frames skipped mlt_producer_get_frame, so _producer was
unset and Movit keyed the chain with a NULL unique id. Pulling
mlt_image_movit from another producer also shared an in-progress GPU
chain onto the cut.

Finalize that chain to rgba64 before sharing, do not copy producer-keyed
_movit state across the boundary, and upload rgba64 as GL_UNSIGNED_SHORT
so 10-bit OpenGL filters on an Adjustment Clip are not distorted.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cross-producer sharing can still expose an incomplete Movit chain, and same-producer paths incur unnecessary GPU round trips.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes Movit crashes and color distortion when processing fx_cut adjustment clips.

Changes:

  • Associates fx_cut frames with their producer.
  • Finalizes cross-producer Movit chains as 16-bit RGBA.
  • Adds safer Movit chain lookups and null handling.
File summaries
File Description
src/modules/movit/mlt_movit_input.cpp Uploads rgba64 as 16-bit data.
src/modules/movit/filter_movit_convert.cpp Hardens chain lookup and producer handling.
src/framework/mlt_producer.c Handles null cut-parent inputs.
src/framework/mlt_playlist.c Sets the producer on fx_cut frames.
src/framework/mlt_frame.c Controls Movit state sharing across producers.
NEWS Documents the fixes.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/framework/mlt_frame.c Outdated
Comment thread src/framework/mlt_frame.c Outdated
Comment thread src/framework/mlt_playlist.c
…chain

Do not hand an in-progress Movit chain back across get_image_with_fx_cut.
Only flatten mlt_image_movit when original producers differ, so single-track
tractor prepends keep the GPU chain. Store FlatInput's GL type so rgba64
upload and set_pixel_data stay in lockstep. Add a movit.saturation fx_cut
test that QSKIPs when the module is unavailable.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new regression test always skips because its fixture never initializes the required GLSL manager and OpenGL context.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/tests/test_tractor/test_tractor.cpp Outdated
@ddennedy ddennedy added this to the v7.42.0 milestone Sep 12, 2026
@ddennedy
ddennedy merged commit 92ce584 into master Sep 12, 2026
15 checks passed
@ddennedy
ddennedy deleted the fix-fx-cut-movit branch September 12, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants