From 832975096620e10157288cc8a69a53eb4adeb60d Mon Sep 17 00:00:00 2001 From: yuhao Date: Mon, 20 Apr 2026 11:58:37 +0000 Subject: [PATCH] Fix Shotcut melt rendering and inclusive timeline timing --- .../cli_anything/shotcut/core/timeline.py | 32 +++++-- .../cli_anything/shotcut/tests/test_core.py | 94 ++++++++++++++++++- .../shotcut/tests/test_full_e2e.py | 38 +++++++- .../cli_anything/shotcut/utils/mlt_xml.py | 51 ++++++++-- 4 files changed, 195 insertions(+), 20 deletions(-) diff --git a/shotcut/agent-harness/cli_anything/shotcut/core/timeline.py b/shotcut/agent-harness/cli_anything/shotcut/core/timeline.py index 13e453367..adbaad099 100644 --- a/shotcut/agent-harness/cli_anything/shotcut/core/timeline.py +++ b/shotcut/agent-harness/cli_anything/shotcut/core/timeline.py @@ -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 diff --git a/shotcut/agent-harness/cli_anything/shotcut/tests/test_core.py b/shotcut/agent-harness/cli_anything/shotcut/tests/test_core.py index 0123972f5..61f79e9aa 100644 --- a/shotcut/agent-harness/cli_anything/shotcut/tests/test_core.py +++ b/shotcut/agent-harness/cli_anything/shotcut/tests/test_core.py @@ -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") diff --git a/shotcut/agent-harness/cli_anything/shotcut/tests/test_full_e2e.py b/shotcut/agent-harness/cli_anything/shotcut/tests/test_full_e2e.py index 74bd66e99..dbacf7be4 100644 --- a/shotcut/agent-harness/cli_anything/shotcut/tests/test_full_e2e.py +++ b/shotcut/agent-harness/cli_anything/shotcut/tests/test_full_e2e.py @@ -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"]) diff --git a/shotcut/agent-harness/cli_anything/shotcut/utils/mlt_xml.py b/shotcut/agent-harness/cli_anything/shotcut/utils/mlt_xml.py index c6e8b25e6..c673dadb5 100644 --- a/shotcut/agent-harness/cli_anything/shotcut/utils/mlt_xml.py +++ b/shotcut/agent-harness/cli_anything/shotcut/utils/mlt_xml.py @@ -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