Fix Shotcut melt rendering and inclusive timeline timing

2026-08-28 23:27:04 +08:00 · 2026-04-20 11:58:37 +00:00
parent bd06b1e1d4
commit 8329750966
4 changed files with 195 additions and 20 deletions
@@ -40,7 +40,11 @@ def _entry_duration_frames(session: Session, entry: dict) -> int:
    out_point = entry.get("out")
    if not out_point:
        raise RuntimeError("Absolute timeline placement requires clips with finite out points")
-    return parse_time_input(out_point, fps_num, fps_den) - parse_time_input(in_point, fps_num, fps_den)
+    return (
+        parse_time_input(out_point, fps_num, fps_den)
+        - parse_time_input(in_point, fps_num, fps_den)
+        + 1
+    )


 def _absolute_insertion_point(
@@ -312,7 +316,7 @@ def remove_clip(session: Session, track_index: int, clip_index: int,
                    fps_num, fps_den = _get_fps(session)
                    in_frames = parse_time_input(in_tc, fps_num, fps_den)
                    out_frames = parse_time_input(out_tc, fps_num, fps_den)
-                    duration_frames = out_frames - in_frames
+                    duration_frames = out_frames - in_frames + 1
                    if duration_frames > 0:
                        duration_tc = frames_to_timecode(duration_frames, fps_num, fps_den)
                        blank = etree.Element("blank")
@@ -454,9 +458,20 @@ def split_clip(session: Session, track_index: int, clip_index: int,
                old_out = child.get("out")
                if old_out is None:
                    raise RuntimeError("Cannot split clip without out point")
+                fps_num, fps_den = _get_fps(session)
+                old_in_frames = parse_time_input(old_in, fps_num, fps_den)
+                old_out_frames = parse_time_input(old_out, fps_num, fps_den)
+                split_frames = parse_time_input(at, fps_num, fps_den)
+                if split_frames <= old_in_frames:
+                    raise ValueError("Split point must be after the clip in point")
+                if split_frames > old_out_frames:
+                    raise ValueError("Split point must not exceed the clip out point")

-                # First part: original in → split point
-                child.set("out", at)
+                first_out = frames_to_timecode(split_frames - 1, fps_num, fps_den)
+
+                # MLT uses inclusive out-points, so the first half must end on
+                # the frame immediately before the split point.
+                child.set("out", first_out)

                # Second part: split point → original out
                # Create a copy of the producer
@@ -470,10 +485,9 @@ def split_clip(session: Session, track_index: int, clip_index: int,
                mlt_xml.set_property(new_producer, "shotcut:uuid",
                                     __import__("uuid").uuid4().hex)

-                # Insert producer in document
-                tractor = session.get_main_tractor()
-                tractor_idx = list(session.root).index(tractor)
-                session.root.insert(tractor_idx, new_producer)
+                # Insert producer ahead of all playlists/tractors so written
+                # MLT never forward-references media producers.
+                mlt_xml.insert_before_playlists_and_tractors(session.root, new_producer)

                # Insert new entry after current one
                new_entry = etree.Element("entry")
@@ -491,7 +505,7 @@ def split_clip(session: Session, track_index: int, clip_index: int,
                    "track_index": track_index,
                    "clip_index": clip_index,
                    "at": at,
-                    "first_clip": {"producer": producer_id, "in": old_in, "out": at},
+                    "first_clip": {"producer": producer_id, "in": old_in, "out": first_out},
                    "second_clip": {"producer": new_prod_id, "in": at, "out": old_out},
                }
            entry_count += 1
@@ -130,6 +130,40 @@ class TestMltXml:
        finally:
            os.unlink(tmpfile)

+    def test_write_mlt_normalizes_producer_order(self):
+        profile = {
+            "width": "1280", "height": "720",
+            "frame_rate_num": "24000", "frame_rate_den": "1001",
+            "sample_aspect_num": "1", "sample_aspect_den": "1",
+            "display_aspect_num": "16", "display_aspect_den": "9",
+            "progressive": "1", "colorspace": "709",
+        }
+        root = create_blank_project(profile)
+        late_producer = root.makeelement("producer", {"id": "late_producer"})
+        set_property(late_producer, "resource", "/tmp/fake.mp4")
+        set_property(late_producer, "mlt_service", "avformat")
+        root.append(late_producer)
+
+        with tempfile.NamedTemporaryFile(suffix=".mlt", delete=False) as f:
+            tmpfile = f.name
+
+        try:
+            write_mlt(root, tmpfile)
+            parsed = parse_mlt(tmpfile)
+            children = list(parsed)
+            first_playlist_or_tractor = min(
+                idx
+                for idx, child in enumerate(children)
+                if child.tag in ("playlist", "tractor")
+            )
+            late_idx = next(
+                idx for idx, child in enumerate(children)
+                if child.tag == "producer" and child.get("id") == "late_producer"
+            )
+            assert late_idx < first_playlist_or_tractor
+        finally:
+            os.unlink(tmpfile)
+
    def test_properties(self):
        from lxml import etree
        elem = etree.Element("producer")
@@ -422,6 +456,40 @@ class TestTimeline:
        finally:
            os.unlink(tmpfile)

+    def test_add_clip_at_time_uses_inclusive_duration(self):
+        s = self._make_session()
+        tl_mod.add_track(s, "video")
+
+        with tempfile.NamedTemporaryFile(suffix=".mp4", delete=False) as f:
+            f.write(b"dummy")
+            tmpfile = f.name
+
+        try:
+            tl_mod.add_clip(
+                s,
+                tmpfile,
+                1,
+                in_point="00:00:00.000",
+                out_point="00:00:01.000",
+            )
+            next_start = frames_to_timecode(timecode_to_frames("00:00:01.000") + 1)
+            tl_mod.add_clip(
+                s,
+                tmpfile,
+                1,
+                in_point="00:00:01.001",
+                out_point="00:00:02.000",
+                at_time=next_start,
+            )
+
+            items = tl_mod.list_clips(s, 1)
+            blank_items = [item for item in items if item.get("type") == "blank"]
+            clip_items = [item for item in items if item.get("clip_index") is not None]
+            assert len(blank_items) == 0
+            assert len(clip_items) == 2
+        finally:
+            os.unlink(tmpfile)
+
    def test_remove_clip(self):
        s = self._make_session()
        tl_mod.add_track(s, "video")
@@ -474,7 +542,8 @@ class TestTimeline:
                            in_point="00:00:00.000", out_point="00:00:10.000")
            result = tl_mod.split_clip(s, 1, 0, "00:00:05.000")
            assert result["action"] == "split_clip"
-            assert result["first_clip"]["out"] == "00:00:05.000"
+            expected_first_out = frames_to_timecode(timecode_to_frames("00:00:05.000") - 1)
+            assert result["first_clip"]["out"] == expected_first_out
            assert result["second_clip"]["in"] == "00:00:05.000"

            # Should now have 2 clips
@@ -484,6 +553,29 @@ class TestTimeline:
        finally:
            os.unlink(tmpfile)

+    def test_remove_clip_without_ripple_preserves_inclusive_duration(self):
+        s = self._make_session()
+        tl_mod.add_track(s, "video")
+
+        with tempfile.NamedTemporaryFile(suffix=".mp4", delete=False) as f:
+            f.write(b"dummy")
+            tmpfile = f.name
+
+        try:
+            tl_mod.add_clip(
+                s,
+                tmpfile,
+                1,
+                in_point="00:00:00.000",
+                out_point="00:00:01.000",
+            )
+            tl_mod.remove_clip(s, 1, 0, ripple=False)
+            items = tl_mod.list_clips(s, 1)
+            blank = next(item for item in items if item.get("type") == "blank")
+            assert parse_time_input(blank["length"]) == timecode_to_frames("00:00:01.000") + 1
+        finally:
+            os.unlink(tmpfile)
+
    def test_move_clip(self):
        s = self._make_session()
        tl_mod.add_track(s, "video", "V1")
@@ -11,7 +11,9 @@ import json
 import copy
 import tempfile
 import shutil
+import subprocess
 import pytest
+from PIL import Image, ImageStat

 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))

@@ -324,7 +326,7 @@ class TestTimelineClips:
        result = tl_mod.split_clip(session, 1, 0, "00:00:05.000")

        assert result["first_clip"]["in"] == "00:00:00.000"
-        assert result["first_clip"]["out"] == "00:00:05.000"
+        assert result["first_clip"]["out"] == frames_to_timecode(parse_time_input("00:00:05.000") - 1)
        assert result["second_clip"]["in"] == "00:00:05.000"
        assert result["second_clip"]["out"] == "00:00:10.000"

@@ -1361,6 +1363,40 @@ class TestMeltRenderE2E:
            assert size > 0
            print(f"\n  MLT XML render: {output_path} ({size:,} bytes)")

+    def test_render_imported_media_is_not_black(self, video):
+        """Imported media should render real picture content, not a black frame."""
+        if shutil.which("ffmpeg") is None:
+            pytest.skip("ffmpeg is required for frame extraction")
+
+        s = Session()
+        proj_mod.new_project(s, "hd1080p30")
+        tl_mod.add_track(s, "video", "V1")
+        tl_mod.add_clip(s, video, 1, "00:00:00.000", "00:00:01.000")
+
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            output_path = os.path.join(tmp_dir, "render.mp4")
+            frame_path = os.path.join(tmp_dir, "frame.png")
+
+            result = export_mod.render(s, output_path, "default", overwrite=True)
+            assert result["method"] == "melt"
+            assert os.path.exists(output_path)
+
+            subprocess.run(
+                [
+                    "ffmpeg", "-y",
+                    "-ss", "00:00:00.500",
+                    "-i", output_path,
+                    "-frames:v", "1",
+                    frame_path,
+                ],
+                check=True,
+                capture_output=True,
+                text=True,
+                timeout=120,
+            )
+            mean = ImageStat.Stat(Image.open(frame_path).convert("RGB")).mean
+            assert max(mean) > 5, f"Rendered frame appears black: {mean}"
+

 if __name__ == "__main__":
    pytest.main([__file__, "-v", "--tb=short"])
@@ -23,8 +23,14 @@ def parse_mlt(filepath: str) -> etree._Element:


 def write_mlt(root: etree._Element, filepath: str) -> None:
-    """Write an MLT XML tree to a file."""
-    tree = etree.ElementTree(root)
+    """Write an MLT XML tree to a file.
+
+    Normalizes top-level producer ordering first so playlist entries never
+    forward-reference media producers that appear later in the document.
+    """
+    normalized_root = copy.deepcopy(root)
+    normalize_top_level_order(normalized_root)
+    tree = etree.ElementTree(normalized_root)
    tree.write(filepath, xml_declaration=True, encoding="utf-8",
               pretty_print=True)

@@ -174,6 +180,39 @@ def create_blank_project(profile: dict) -> etree._Element:
    return root


+def _first_playlist_or_tractor_index(root: etree._Element) -> int:
+    """Return the first top-level playlist/tractor index, or len(root)."""
+    for idx, child in enumerate(list(root)):
+        if child.tag in ("playlist", "tractor"):
+            return idx
+    return len(root)
+
+
+def insert_before_playlists_and_tractors(
+    root: etree._Element, element: etree._Element
+) -> None:
+    """Insert a top-level declaration before any playlists or tractors."""
+    root.insert(_first_playlist_or_tractor_index(root), element)
+
+
+def normalize_top_level_order(root: etree._Element) -> None:
+    """Move any late top-level producers ahead of playlists and tractors."""
+    late_producers = []
+    seen_playlist_or_tractor = False
+    for child in list(root):
+        if child.tag in ("playlist", "tractor"):
+            seen_playlist_or_tractor = True
+        elif child.tag == "producer" and seen_playlist_or_tractor:
+            late_producers.append(child)
+
+    for producer in late_producers:
+        root.remove(producer)
+
+    insert_idx = _first_playlist_or_tractor_index(root)
+    for offset, producer in enumerate(late_producers):
+        root.insert(insert_idx + offset, producer)
+
+
 def add_track_to_tractor(root: etree._Element, tractor: etree._Element,
                         track_type: str = "video",
                         name: str = "") -> tuple[str, str]:
@@ -290,13 +329,7 @@ def create_producer(root: etree._Element, resource: str,
    # Generate a UUID for clip tracking
    set_property(producer, "shotcut:uuid", str(uuid.uuid4()))

-    # Insert before the first tractor
-    tractors = root.findall("tractor")
-    if tractors:
-        tractor_idx = list(root).index(tractors[0])
-        root.insert(tractor_idx, producer)
-    else:
-        root.append(producer)
+    insert_before_playlists_and_tractors(root, producer)

    return producer