Skip to content

Fix timeline operations failing when clips share a producer#1816

Open
abcfy2 wants to merge 1 commit intomltframework:masterfrom
abcfy2:fix/shared-producer-uuid-collision
Open

Fix timeline operations failing when clips share a producer#1816
abcfy2 wants to merge 1 commit intomltframework:masterfrom
abcfy2:fix/shared-producer-uuid-collision

Conversation

@abcfy2
Copy link
Copy Markdown

@abcfy2 abcfy2 commented Apr 23, 2026

Summary

  • Fix wrong clip deletion: When multiple playlist entries reference the same MLT producer (e.g. projects generated by external tools that reuse a single chain element), removeSelection() and liftSelection() went through a UUID round-trip (selectionUuids()findClipByUuid()) that always returned the first matching clip instead of the selected one. Fixed by using direct clip indices from the existing selection, sorted in descending order to avoid index shift during ripple delete.
  • Fix undo corruption: UndoHelper::recordBeforeState() used clip->parent() UUID as the dictionary key in m_state. When all entries share the same parent producer, all entries had the same UUID, causing state records to overwrite each other — only the last clip's info survived. Undo then restored the entire track as copies of the last clip. Fixed by using the cut UUID (unique per playlist entry) instead of the parent UUID.
  • Preserve trim keyframe restore: The m_xmlClips lookup in recordBeforeState() also checks the parent UUID to maintain compatibility with trim commands that register via storeXmlForClip(parentUuid) under SkipXML hint.

Test plan

  • Open a project with multiple clips sharing the same producer on a track
  • Select clip N (not the first), press Delete → verify clip N is removed, not clip 0
  • Press Ctrl+Z → verify the correct clip is restored at the correct position with correct in/out
  • Select multiple non-contiguous clips, press Delete → verify all selected clips are removed
  • Select multiple clips, press Lift (Delete without ripple) → verify all selected clips are replaced with blanks
  • Trim a clip with keyframes, undo → verify keyframes are restored correctly
  • Normal workflow: add clips from playlist to timeline, edit, undo/redo → verify no regression

@abcfy2
Copy link
Copy Markdown
Author

abcfy2 commented Apr 23, 2026

Background

cli-anything-shotcut generates timeline projects where clips from the same source video all reference a single chain element in the MLT XML, instead of creating a separate producer per clip:

<chain id="tl_clip0" ...>  <!-- one shared chain -->
<playlist id="playlist_6dcbc856">
    <entry producer="tl_clip0" in="00:00:14.820" out="00:00:15.580" />
    <entry producer="tl_clip0" in="00:00:16.260" out="00:00:18.020" />
    <entry producer="tl_clip0" in="00:00:18.940" out="00:00:20.140" />
    ...3000+ entries all referencing the same tl_clip0
</playlist>

This is fully valid MLT — both Kdenlive and melt handle it correctly. It provides significant performance benefits: shared demuxer/decoder, fast project load, minimal memory usage.

However, this triggers two bugs in Shotcut:

Bug 1: Wrong clip deleted

Select clip #3, press Delete → clip #1 is removed instead.

Screenshot from 2026-04-23 09-24-36

Result after Delete:

Screenshot from 2026-04-23 09-24-52

Bug 2: Undo corruption

Press Ctrl+Z to undo → the entire track becomes copies of the last clip.

Screenshot from 2026-04-23 09-38-02

Root cause

Shotcut identifies clips by producer UUID (clip->parent()), but when entries share a producer they all have the same UUID. This causes findClipByUuid() to always return the first match, and UndoHelper::m_state (a QMap<QUuid, Info>) to overwrite entries so only the last clip's info survives.

@ddennedy
Copy link
Copy Markdown
Member

Shotcut does not try to support anything you can write in MLT XML, only what it writes. There is a reason it does not reuse producers that has to do with avformat producer internals especially with respect to audio. I am not certain we want to try to maintain this change since there is no good way to test it.

What is the point of cli-anything when MLT already has a CLI and python bindings?

@abcfy2
Copy link
Copy Markdown
Author

abcfy2 commented Apr 23, 2026

Hi @ddennedy,

Thanks for the feedback! Let me address your questions:

About cli-anything-shotcut

While MLT does have a CLI and Python bindings, the challenge is that not all valid MLT XML can be re-edited in Shotcut — unsupported MLT structures will typically just be recognized as a single media source rather than an editable project. That's the value cli-anything-shotcut provides: it generates Shotcut-friendly MLT project files that users can continue editing. The same rationale applies to cli-anything-kdenlive.

I've recently been maintaining and improving both CLIs, fixing quite a few compatibility issues along the way. Through that work I've learned firsthand that you can't simply use the MLT CLI to create a project file that's fully editable in Shotcut or Kdenlive — there are specific conventions and properties these editors expect that raw MLT output doesn't include. cli-anything-shotcut exists specifically to bridge that gap.

About the fix itself

I'd also point out that this change is actually a net simplification. The original code in removeSelection()/liftSelection() performed an unnecessary indirection: selection() already returns the clip indices (trackIndex, clipIndex) as QPoints, but the code converted them to UUIDs via selectionUuids(), then converted them back to indices via findClipByUuid(). All downstream operations — RemoveCommand, LiftCommand, and ultimately m_model.removeClip() / m_model.liftClip() — work purely with (trackIndex, clipIndex). The MLT playlist API itself operates by index (playlist.remove(clipIndex), playlist.insert(...), etc.), so the UUID round-trip was redundant overhead that also happened to break when entries share a producer. The fix simply uses the indices directly.

About testing

I understand the concern about testability. Unfortunately Shotcut doesn't have C++ unit tests, and the existing JS tests only cover EDL/chapters export. That said, the changes are well-isolated: the timelinedock.cpp change is a straightforward index-based replacement, and the undohelper.cpp change simply shifts the UUID source from parent to cut. I've included a manual test plan in the PR description covering the key scenarios.

@ddennedy
Copy link
Copy Markdown
Member

ddennedy commented Apr 23, 2026

Ok, thanks, I appreciate your point about cli-anything creating editor-compatible projects. It is very good if AI can generate things that can be more easily edited with a tool instead of only trial-and-error adjustment prompts. Just like code generation!

It is true that producer reuse can be quite helpful not only for internal efficiency but for user to more easily modify a single set of filters, for example. I think we will want to include it, and then add intelligence to not reuse for the rare, known problem situations.

i will look into this PR soon after the release next week. Also planning to start adding unit tests next month.

When multiple playlist entries reference the same producer (e.g. generated
by external tools that reuse a single chain element), Shotcut's UUID-based
clip identification breaks because all entries share the same producer UUID.

This causes:
- Delete/Lift removes the wrong clip (always the first match)
- Undo restores the entire track as copies of the last clip

Fix by:
1. removeSelection()/liftSelection(): use direct clip indices from the
   existing selection instead of the UUID round-trip. Sort descending
   by (trackIndex, clipIndex) to avoid index shift during ripple delete.
2. UndoHelper: use cut UUID (unique per playlist entry) instead of parent
   UUID (shared across entries) as the dictionary key, preventing state
   entries from overwriting each other. Also check parent UUID in
   m_xmlClips lookup to preserve the trim XML restore path for keyframes.
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.

2 participants