From 16ce50d5ae294c898b6c2992555a1febd58d9b52 Mon Sep 17 00:00:00 2001 From: Matt Gradwohl <3945134+mgradwohl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:09:00 -0600 Subject: [PATCH 1/5] Save linked movie clip ranges without dropping source frames --- README.md | 5 +- docs/migration-plan.md | 4 +- docs/native-format.md | 21 ++++- src/tracker_writer.cpp | 36 ++++---- src/tracker_writer.hpp | 4 +- tests/corpus/README.md | 25 ++++++ tests/corpus/manifest.json | 1 + tests/corpus_probe.cpp | 10 ++- tests/paired_workspace_test.cpp | 150 +++++++++++++++++++------------- tests/test_corpus.py | 91 +++++++++++++++++-- tests/tracker_writer_test.cpp | 49 ++++++++++- tools/corpus.py | 68 ++++++++++++--- 12 files changed, 354 insertions(+), 110 deletions(-) diff --git a/README.md b/README.md index b01c754..a5a51e2 100644 --- a/README.md +++ b/README.md @@ -71,14 +71,15 @@ then write your current edits. Changes made by another program are preserved and conflict. Recovery never marks newer edits saved without writing them to both files. This workflow currently covers audited media-free and local-image experiments, plus selected-member -updates to existing image archives. The first linked-movie writer supports mass and mark edits in +updates to existing image archives. The linked-movie writer supports mass, mark and clip-range edits in audited constant-rate imports after timestamp indexing; its edited-file Tracker save/reopen acceptance has passed for the pinned four-frame AVI case. Later revisions still require their own CI and review. Movie timing/calibration/track-identity edits, movie packaging, filters and broader settings remain unfinished. An unsupported export is refused; save a MoTrack copy to retain work. Image experiments can retain Clip Settings changes to the starting frame, ending frame and frame interval in linked `.trk` files, existing image archives and portable image exports. All source images and marks remain available, including those outside the selected clip; image timing stays unchanged. -The new range-edit cases still require actual Tracker application acceptance before merge. +Audited linked movies also retain Clip Settings range and interval edits after indexing. The complete +movie and all measurements stay available; source frame numbers and numeric timing are unchanged. The [format guide](docs/native-format.md) records the exact supported edits and compatibility evidence. ### Export a portable image experiment diff --git a/docs/migration-plan.md b/docs/migration-plan.md index 1e24bd0..77a9ca4 100644 --- a/docs/migration-plan.md +++ b/docs/migration-plan.md @@ -168,9 +168,9 @@ and platform evidence before it can be marked complete. | --- | --- | --- | --- | | R01 | Reproducible native builds, CI, review, numerical tests, at least 90% combined C++ line coverage | Implemented foundation; continue applying gates to every new component. The reference helper executes repeated loader completion callbacks once, fixing the empty-resave race in #105; bounded failure diagnostics remain and no retry or acceptance bypass is added. | 0, every phase | | R02 | Open `.trk`/`.trz`; preserve all supported and unknown content, references, metadata, and archive tabs | Partial: supported point masses/vectors/calibration import; one archive tab selected. Native documents retain that tab's original XML, editable-track IDs and source-object associations for new imports. Version 5 embeds the exact original archive, including all tabs/media/unknown files. A partial read-only reference index maps original track names, panel/nested-view selections, table/plot owners (including dataset rows), ordered plot guests, mechanical-tool attachment slots/step targets center-of-mass/vector-sum members, model boosters and dynamic-system members to source nodes and known IDs; unknown subclass relationships remain explicit. Dataset rows have source-structure evidence only; media-backed application acceptance remains. Older native sources retain an explicit unknown-association state. | 1–3 | -| R03 | Save edited standard files plus optional `.motrack`; reopen in both applications without data loss | Partial: native source/archive preservation and audited standard writers are merged. Depending on the source schema, verified edits include mass/marks, independent vectors, clip ranges, media-free constant timing, fixed/partly fixed/varying calibration, unit labels, audited renames, independent track creation and supported removal/selection/plot/table cleanup. The [format guide](native-format.md) defines those per-schema limits. Linked images, selected image-archive members and indexed constant-rate linked movies have native reopen/relocation and actual Tracker acceptance. Per-output checkpoints, explicit recovery, desktop paired Save and portable image export are merged through #133. Image clip ranges/strides are merged in #134 with all four linked/archive/package application cases, including complete post-resave image inventory checks. Untouched archive experiments are checked independently. Remaining work includes broader schemas and dependencies, dataset-owner removal, movie archives/packaging, broader media timing and full export authoring. Known Tracker table-format losses (#64) and vector-sum losses (#63) retain strict guards and independent failing expectations. | 2–3, every feature | +| R03 | Save edited standard files plus optional `.motrack`; reopen in both applications without data loss | Partial: native source/archive preservation and audited standard writers are merged. Depending on the source schema, verified edits include mass/marks, independent vectors, clip ranges, media-free constant timing, fixed/partly fixed/varying calibration, unit labels, audited renames, independent track creation and supported removal/selection/plot/table cleanup. The [format guide](native-format.md) defines those per-schema limits. Linked images, selected image-archive members and indexed constant-rate linked movies have native reopen/relocation and actual Tracker acceptance. Per-output checkpoints, explicit recovery, desktop paired Save and portable image export are merged through #133. Image clip ranges/strides are merged in #134 with all four linked/archive/package application cases, including complete post-resave image inventory checks. The next movie clip-range slice has local native/desktop/export checks and a complete post-resave movie-hash oracle; its actual Tracker acceptance and current-head CI/review remain pending. Untouched archive experiments are checked independently. Remaining work includes broader schemas and dependencies, dataset-owner removal, movie archives/packaging, broader media timing and full export authoring. Known Tracker table-format losses (#64) and vector-sum losses (#63) retain strict guards and independent failing expectations. | 2–3, every feature | | R04 | Multiple experiment tabs, independent undo/dirty state, tab/project Save As, cross-tab import and reusable views | Partial foundation: document sessions own independent data/history/save checkpoints, and native storage preserves unopened archive tabs. The desktop still has one active experiment; multi-tab controllers/UI, cross-tab import and reusable views remain. | 2–4 | -| R05 | Movies, stills, image sequences and zipped sequences; accurate clip/time/frame controls and media properties | Partial: local movies/images, trim/stride, indexing, orientation, bounded caching and playback exist. Media-free duration/time-origin edits have history and export oracles. Four-image and four-frame constant-rate AVI fixtures independently check pixels, order, timestamps, clip time and calibrated marks. The movie baseline (#132) passes actual Tracker load/save/fresh reopen; existing mass/mark export (#133) also passes native reimport; movie edits require unchanged timing/calibration/identity and a matching completed native index. Image clip edits (#134) pass all four linked/archive/package cases while preserving source frame numbers, measurements and every image, including excluded frames. See the merged CI table above. Variable-rate timing, movie archives, broader formats/properties/controls, numeric media timing edits and media export remain. The synthetic AVI does not reproduce or resolve the historical MOV/Open Recent report in #65. | 1, 3–4, 6 | +| R05 | Movies, stills, image sequences and zipped sequences; accurate clip/time/frame controls and media properties | Partial: local movies/images, trim/stride, indexing, orientation, bounded caching and playback exist. Media-free duration/time-origin edits have history and export oracles. Four-image and four-frame constant-rate AVI fixtures independently check pixels, order, timestamps, clip time and calibrated marks. The movie baseline (#132) passes actual Tracker load/save/fresh reopen; existing mass/mark export (#133) also passes native reimport; movie edits require unchanged timing/calibration/identity and a matching completed native index. Image clip edits (#134) pass all four linked/archive/package cases while preserving source frame numbers, measurements and every image, including excluded frames. See the merged CI table above. Strided/single-frame movie range saving now has local native/desktop tests and two new reference modes pending actual Tracker acceptance. Variable-rate timing, movie archives, broader formats/properties/controls, numeric media timing edits and media export remain. The synthetic AVI does not reproduce or resolve the historical MOV/Open Recent report in #65. | 1, 3–4, 6 | | R06 | Complete point-mass editing and appearance: selection, dragging/nudging, numeric positions, multiselect, locks, labels, trails, paths, and vector overlays | Partial: manual marking/replacement, repeat, rename/delete, mass, and undo exist. Finish manipulation, appearance, path length, and selection behavior. | 4–5 | | R07 | Independent vectors, linked vectors, sums, their data, editing, and persistence | Partial: independent vectors import/display/edit/undo/native-save. Internal sample export, new-vector creation and unreferenced removal have independent empty/sparse/zero-vector reference cases; native selection-cleanup regressions cover empty and replacement vectors; creation retains the snapping/precision guards and requires an audited independent-vector/axes source. A Java-authored vector-sum case retains independent expected results but reproduces a Tracker 6.3.5 defect: members never resolve and resave loses their names. This is explicitly not accepted round-trip evidence; the internal writer rejects edited projects containing vector sums, including empty member tracks. Native read-only indexing now records original member links with explicit ambiguity/unclassified targets. Native links/sum evaluation, analysis and desktop standard saving remain. | 3–5 | | R08 | Center-of-mass tracks, constituents, dependent updates, and moving reference frames | Native evaluation/editing missing. A Java-authored center-of-mass reference checks named constituents, total mass and independent weighted positions; native member-link indexing and constituent-name export preserve original targets. The rename case requires unchanged 5 kg total mass and weighted positions through actual Tracker resave; native dependency evaluation, constituent editing and moving reference frames remain. | 2, 5 | diff --git a/docs/native-format.md b/docs/native-format.md index 0edba88..a153186 100644 --- a/docs/native-format.md +++ b/docs/native-format.md @@ -650,9 +650,9 @@ Windows/macOS and actual Tracker package acceptance, with 95.72% combined C++ li ### Linked constant-rate movie edits The document writer accepts the explicitly audited `XuggleVideo`/`VideoClipControl` schema only through -its movie adapter. Existing point-mass values and image-coordinate marks, plus the linked media path, -may change. Original names, membership, calibration, unit labels, clip timing/range and other parsed -fields must remain unchanged. Filters, vectors, other track types except axes, display-orientation +its movie adapter. Existing point-mass values and image-coordinate marks, clip start/step/count and the +linked media path may change. Original names, membership, calibration, unit labels, numeric clip timing +and other parsed fields must remain unchanged. Filters, vectors, other track types except axes, display-orientation transforms and archived movies remain refused. The archive entry point rejects movie documents before clearing archive metadata for its internal snapshot; this also applies after indexing and native reopen. This opt-in schema does not enable movie renames or @@ -667,7 +667,9 @@ index limit. Clip endpoints and marks must fit that index. The native runtime fr the complete index or the clip endpoint inferred on import; it is removed from the export snapshot, not written as a standard-file edit. Existing saved `video_framecount`, when present, must match the index. Decoded timestamps stay in seconds; preserved XML clip time origin and interval use milliseconds. -Native movie analysis remains aligned to the imported clip's time origin. +Native movie analysis remains aligned to the imported clip's time origin. Range edits retain every +original-frame mark and the full movie index; no frames are renumbered or movie bytes rewritten. The +shared clip writer retains its 500000 last-frame bound and checks Tracker's temporary Java-int count. Movie links use the same physical-directory, retained-location and atomic-write checks as images. Preflight verifies current readability and identity, not general codec compatibility or protection @@ -686,3 +688,14 @@ and review at d9a24c6 in [PR #133](https://github.com/mgradwohl/MoTrack/pull/133 [run 42549945960](https://github.com/mgradwohl/MoTrack/actions/runs/42549945960), including the edited Tracker movie case and 95.71% combined coverage. This scoped acceptance does not establish full movie interchange or resolve #65. + + +The movie-range extension adds strided and single-frame native export/reopen cases, desktop Clip +Settings/undo/redo/paired-save checks and two actual-Tracker acceptance modes. Independent whole-XML +expectations permit only the requested range, mass, mark and path; every mark outside the clip remains. +After Tracker saves and freshly reopens, the saved movie reference must resolve locally to the complete +pinned byte count/SHA-256, including excluded frames. Local Windows validation passes all nine CTest +groups (124.80 seconds), 86 Python tests, both native export/oracle/reimport cases, formatting and +static analysis for all four changed C++ source files, with 95.64% line coverage (8838/9241). Three +existing LLVM function-profile mismatch warnings remain visible. Current-head three-platform CI, +actual Tracker movie-range acceptance and review remain required before merging this extension. diff --git a/src/tracker_writer.cpp b/src/tracker_writer.cpp index 6541d4d..23e3ed0 100644 --- a/src/tracker_writer.cpp +++ b/src/tracker_writer.cpp @@ -129,14 +129,21 @@ void patchUnit(QDomDocument& xml, const QString& name, const std::string& unit) patchScalar(xml, xml.documentElement(), name, "string", label); } -void patchClipRange(QDomDocument& xml, const Clip& clip, bool linkedImages) +enum class LinkedMedia : std::uint8_t +{ + None, + Images, + Movie +}; + +void patchClipRange(QDomDocument& xml, const Clip& clip, LinkedMedia media) { if (clip.frameDurationMs) { auto timing = clip; - // Audited image sequences use the same constant interval arithmetic. This private + // Audited media use the same constant interval arithmetic. This private // numeric check does not change the saved source inventory or enable timing edits. - if (linkedImages) + if (media != LinkedMedia::None) timing.mediaReferences.clear(); require(validConstantClipTiming(timing)); } @@ -152,12 +159,13 @@ void patchClipRange(QDomDocument& xml, const Clip& clip, bool linkedImages) require(property.attribute("type") == "object" && object.attribute("class") == "org.opensourcephysics.media.core.VideoClip" && object.nextSiblingElement().isNull()); require(uniqueProperty(object, "frameshift").isNull()); - if (linkedImages) + if (media != LinkedMedia::None) { - // The image schema, complete source-frame inventory and optional saved count were - // checked before patching. Trimming changes only the clip, never the image inventory. + // The media schema, complete frame inventory/index and optional saved count were + // checked before patching. Trimming changes only the clip, never the source media. require(uniqueProperty(object, "video").firstChildElement("object").attribute("class") == - "org.opensourcephysics.media.core.ImageVideo"); + (media == LinkedMedia::Images ? "org.opensourcephysics.media.core.ImageVideo" + : "org.opensourcephysics.media.xuggle.XuggleVideo")); } else require(uniqueProperty(object, "video").isNull() && uniqueProperty(object, "video_framecount").isNull()); @@ -1006,22 +1014,18 @@ bool patchLinkedImagePaths(QDomDocument& xml, const LegacyProject& project, Lega return true; } -enum class LinkedMedia : std::uint8_t -{ - None, - Images, - Movie -}; - bool patchLinkedMoviePath(QDomDocument& xml, const LegacyProject& project, LegacyProject& baseline, std::span frameTimes) { validateTrackerMovieEditSchema(xml); require(project.clip.mediaReferences.size() == 1 && baseline.clip.mediaReferences.size() == 1 && project.vectors.empty() && std::ranges::all_of(project.unsupportedTrackTypes, [](const auto& type) { return type == "org.opensourcephysics.cabrillo.tracker.CoordAxes"; })); - // The first movie slice changes only existing mass values, marks and the linked path. + // Movie edits retain original timing, geometry and identity. Clip ranges keep source frame numbers. auto unchanged = project; unchanged.clip.mediaReferences = baseline.clip.mediaReferences; + unchanged.clip.firstFrame = baseline.clip.firstFrame; + unchanged.clip.stepSize = baseline.clip.stepSize; + unchanged.clip.stepCount = baseline.clip.stepCount; require(unchanged.pointMasses.size() == baseline.pointMasses.size()); for (std::size_t index = 0; index < unchanged.pointMasses.size(); ++index) { @@ -1227,7 +1231,7 @@ std::string renderTrackerProjectImpl(const LegacyProject& project, if (expected.clip.firstFrame != baseline.project.clip.firstFrame || expected.clip.stepSize != baseline.project.clip.stepSize || expected.clip.stepCount != baseline.project.clip.stepCount) { - patchClipRange(xml, expected.clip, media == LinkedMedia::Images); + patchClipRange(xml, expected.clip, media); baseline.project.clip.firstFrame = expected.clip.firstFrame; baseline.project.clip.stepSize = expected.clip.stepSize; baseline.project.clip.stepCount = expected.clip.stepCount; diff --git a/src/tracker_writer.hpp b/src/tracker_writer.hpp index 7784730..552c13d 100644 --- a/src/tracker_writer.hpp +++ b/src/tracker_writer.hpp @@ -18,9 +18,9 @@ struct WorkspaceDocument; void writeTrackerProject(const QString& path, const LegacyProject& project, const TrackerSource& source); /// Atomically export a standalone document. mediaProjectPath is the absolute current project file path; /// local image references use its physical parent directory, resolving existing filesystem aliases before rebasing. -/// The source/native snapshot remains unchanged. Audited constant-rate linked movies permit existing mass/mark edits +/// The source/native snapshot remains unchanged. Audited constant-rate linked movies permit existing mass/mark and clip-range edits /// after native timestamp indexing; their original timing, calibration and other fields stay unchanged. -/// Archives, filters, movie rename/membership edits and media timing/range edits are refused. +/// Archives, filters, movie rename/membership edits and numeric media timing edits are refused. /// All linked files must be readable. Symbolic-link output files and unsafe generated references are refused. /// No media is copied and no session save checkpoint is changed. void writeTrackerDocument(const QString& path, const WorkspaceDocument& document, const QString& mediaProjectPath); diff --git a/tests/corpus/README.md b/tests/corpus/README.md index 0954b94..c03cb9c 100644 --- a/tests/corpus/README.md +++ b/tests/corpus/README.md @@ -1113,3 +1113,28 @@ rejects incorrect clip bounds/stride/count, raw or displayed time, calibration, and missing marks, even if all reported application stages agree on the wrong values. The final 84-test Python suite and all four actual-Tracker modes passed in the merged revision above. This establishes those audited image ranges only; MoTrack #65 remains open. + + +### Linked-movie clip range and stride acceptance + +The constant-rate movie fixture adds linked-movie-clip (frames 1 through 3, stride 2) and +linked-movie-clip-single (frame 2 only). Each mode uses the native decoder's complete four-frame +index, edits mass and mark 1, changes the clip with undo/redo, saves/reopens the native companion, +removes its original project directory and exports a standard .trk. + +The independent expectations retain original marks 0/1/2 and the complete movie. Displayed times are +-250 ms at frame 1 and 0 ms at frame 3 for the strided clip, or -250 ms at frame 2 for the single-frame +clip. Raw movie timestamps remain 0/125/250/375 ms. Active-bound pixel checks do not extend the live +clip. After Tracker saves and again after a fresh process reopens it, a bounded XML/path/hash check +requires the complete saved movie reference to resolve to the pinned original AVI. Its per-stage +reference, byte count and SHA-256 are recorded as saved_movie_assets. The baseline and ordinary +movie-edit modes use the same post-resave verification. + +The 86 Python tests reject changed range/timing/marks, missing or changed movie bytes and malformed, +remote or archive references; local relative/absolute/file-URL paths have positive coverage. Both +native exports, full XML/movie hashes and native reimport pass locally. All nine Windows CTest groups +pass in 124.80 seconds, with 95.64% line coverage (8838/9241); formatting and all four changed C++ +source-file static analyses pass. Three existing LLVM function-profile mismatch warnings remain visible. +The new modes still require current-head Linux actual Tracker load/save/fresh reopen/native reimport +and three-platform CI before merge. This does not close MoTrack #65 or reproduce the historical MOV/ +Open Recent report; the upstream issue and complete discussion were refreshed for this slice. diff --git a/tests/corpus/manifest.json b/tests/corpus/manifest.json index ea24e48..87b9f61 100644 --- a/tests/corpus/manifest.json +++ b/tests/corpus/manifest.json @@ -1427,6 +1427,7 @@ { "id": "linked-movie", "native_linked_movie_roundtrip": true, + "native_linked_movie_clip_roundtrip": true, "tracker_version": "6.3.5", "file": "linked-movie.trk", "local_file": "fixtures/linked-movie/linked-movie.trk", diff --git a/tests/corpus_probe.cpp b/tests/corpus_probe.cpp index 2afefb6..2432366 100644 --- a/tests/corpus_probe.cpp +++ b/tests/corpus_probe.cpp @@ -234,7 +234,8 @@ try motrack::writeTrackerArchiveDocument(args[3], document); return 0; } - if (args.size() == 4 && args[1] == "--export-linked-movie") + if (args.size() == 4 && + (args[1] == "--export-linked-movie" || args[1] == "--export-linked-movie-clip" || args[1] == "--export-linked-movie-clip-single")) { QTemporaryDir temporary; const auto inputDirectory = temporary.filePath("source"); @@ -264,6 +265,13 @@ try !edits.markPointMass(document.project, 0, {.frame = 1, .x = 8.0, .y = 12.0}) || !edits.undo(document.project) || !edits.redo(document.project)) throw motrack::ProjectError("Could not edit linked-movie reference."); + if (args[1] != "--export-linked-movie") + { + const bool single = args[1] == "--export-linked-movie-clip-single"; + if (!edits.setClipRange(document.project, single ? 2 : 1, single ? 2 : 3, single ? 1 : 2) || !edits.undo(document.project) || + !edits.redo(document.project)) + throw motrack::ProjectError("Could not edit the movie clip range."); + } motrack::rebaseWorkspaceMedia(document, source, native); motrack::writeWorkspaceDocument(native, document); document = {}; diff --git a/tests/paired_workspace_test.cpp b/tests/paired_workspace_test.cpp index bca931f..fccdb15 100644 --- a/tests/paired_workspace_test.cpp +++ b/tests/paired_workspace_test.cpp @@ -183,72 +183,98 @@ TEST(PairedWorkspace, SavesEditedLinkedAndArchivedProjectsAndUsesThePairForOrdin } } -TEST(PairedWorkspace, SavesImageClipSettingsAndReopensTheSameSourceFramesThroughUndoRedo) +TEST(PairedWorkspace, SavesMediaClipSettingsAndReopensTheSameSourceFramesThroughUndoRedo) { - QTemporaryDir temporary; - motrack::MainWindow window; - ASSERT_TRUE(window.loadProject(fixture())); - auto* settings = window.findChild("clipSettings"); - auto* undo = window.findChild("undoEdit"); - auto* redo = window.findChild("redoEdit"); - auto* save = window.findChild("saveProject"); - auto* slider = window.findChild("stepSlider"); - ASSERT_TRUE(settings && undo && redo && save && slider); - ASSERT_TRUE(settings->isEnabled()); - bool accepted = false; - QTimer::singleShot(0, - &window, - [&accepted] - { - if (auto* dialog = qobject_cast(QApplication::activeModalWidget())) + for (const bool movie : {false, true}) + { + SCOPED_TRACE(movie); + const auto source = movie ? QString::fromUtf8(MOTRACK_FIXTURES) + "/../corpus/fixtures/linked-movie/linked-movie.trk" : fixture(); + QTemporaryDir temporary; + motrack::MainWindow window; + ASSERT_TRUE(window.loadProject(source)); + if (movie) + { + auto* plot = window.findChild("positionPlot"); + ASSERT_NE(plot, nullptr); + ASSERT_TRUE(QTest::qWaitFor([plot] { return plot->accessibleName().endsWith("Time (s)"); })); + } + auto* settings = window.findChild("clipSettings"); + auto* undo = window.findChild("undoEdit"); + auto* redo = window.findChild("redoEdit"); + auto* save = window.findChild("saveProject"); + auto* slider = window.findChild("stepSlider"); + ASSERT_TRUE(settings && undo && redo && save && slider); + ASSERT_TRUE(settings->isEnabled()); + bool accepted = false; + QTimer::singleShot(0, + &window, + [&accepted] { - auto* first = dialog->findChild("clipStart"); - auto* stride = dialog->findChild("clipStride"); - if (!first || !stride) + if (auto* dialog = qobject_cast(QApplication::activeModalWidget())) { - dialog->reject(); - return; + auto* first = dialog->findChild("clipStart"); + auto* stride = dialog->findChild("clipStride"); + if (!first || !stride) + { + dialog->reject(); + return; + } + first->setValue(1); + stride->setValue(2); + accepted = true; + dialog->accept(); } - first->setValue(1); - stride->setValue(2); - accepted = true; - dialog->accept(); - } - }); - settings->trigger(); - ASSERT_TRUE(accepted); - ASSERT_TRUE(window.isWindowModified()); - EXPECT_EQ(slider->maximum(), 1); - const auto standard = temporary.filePath("clip.trk"); - const auto native = temporary.filePath("clip.motrack"); - ASSERT_TRUE(window.saveTrackerProject(standard)); - EXPECT_FALSE(window.isWindowModified()); - const auto trimmed = motrack::importTrackerDocument(standard).project; - EXPECT_EQ(trimmed.clip.firstFrame, 1); - EXPECT_EQ(trimmed.clip.stepSize, 2); - EXPECT_EQ(trimmed.clip.stepCount, 2); - EXPECT_EQ(trimmed.pointMasses, motrack::importTrackerDocument(fixture()).project.pointMasses); - undo->trigger(); - EXPECT_TRUE(window.isWindowModified()); - save->trigger(); - EXPECT_FALSE(window.isWindowModified()); - EXPECT_EQ(motrack::importTrackerDocument(standard).project.clip.firstFrame, 0); - redo->trigger(); - save->trigger(); - EXPECT_FALSE(window.isWindowModified()); - EXPECT_EQ(motrack::importTrackerDocument(standard).project, trimmed); - ASSERT_TRUE(window.loadProject(native)); - EXPECT_EQ(slider->maximum(), 1); - const auto reopened = motrack::readWorkspaceDocument(native); - const auto sequence = motrack::ImageSequence::open(native, reopened.project.clip); - if (!sequence) - FAIL() << "The saved clip lost its image sequence."; - for (int step = 0; step < 2; ++step) - { - const auto frame = sequence->readStep(step); - if (!frame) - FAIL() << "The saved clip lost a source frame."; - EXPECT_EQ(frame->image, QImage(fixture("images/frame" + QString::number(1 + step * 2) + ".png"))); + }); + settings->trigger(); + ASSERT_TRUE(accepted); + ASSERT_TRUE(window.isWindowModified()); + EXPECT_EQ(slider->maximum(), 1); + const auto standard = temporary.filePath("clip.trk"); + const auto native = temporary.filePath("clip.motrack"); + ASSERT_TRUE(window.saveTrackerProject(standard)); + EXPECT_FALSE(window.isWindowModified()); + const auto trimmed = motrack::importTrackerDocument(standard).project; + EXPECT_EQ(trimmed.clip.firstFrame, 1); + EXPECT_EQ(trimmed.clip.stepSize, 2); + EXPECT_EQ(trimmed.clip.stepCount, 2); + EXPECT_EQ(trimmed.pointMasses, motrack::importTrackerDocument(source).project.pointMasses); + undo->trigger(); + EXPECT_TRUE(window.isWindowModified()); + save->trigger(); + EXPECT_FALSE(window.isWindowModified()); + EXPECT_EQ(motrack::importTrackerDocument(standard).project.clip.firstFrame, 0); + redo->trigger(); + save->trigger(); + EXPECT_FALSE(window.isWindowModified()); + EXPECT_EQ(motrack::importTrackerDocument(standard).project, trimmed); + ASSERT_TRUE(window.loadProject(native)); + EXPECT_EQ(slider->maximum(), 1); + const auto reopened = motrack::readWorkspaceDocument(native); + if (movie) + { + EXPECT_EQ(reopened.movieFrameTimes, (std::vector{0.0, 0.125, 0.25, 0.375})); + auto* canvas = window.findChild("videoCanvas"); + ASSERT_NE(canvas, nullptr); + for (int step = 0; step < 2; ++step) + { + slider->setValue(step); + ASSERT_TRUE( + QTest::qWaitFor([canvas, step] { return canvas->accessibleName() == "Frame " + QString::number(1 + step * 2); })); + } + } + else + { + const auto sequence = motrack::ImageSequence::open(native, reopened.project.clip); + if (!sequence) + FAIL() << "The saved clip lost its image sequence."; + for (int step = 0; step < 2; ++step) + { + const auto frame = sequence->readStep(step); + if (!frame) + FAIL() << "The saved clip lost a source frame."; + EXPECT_EQ(frame->image, QImage(fixture("images/frame" + QString::number(1 + step * 2) + ".png"))); + } + } } } diff --git a/tests/test_corpus.py b/tests/test_corpus.py index 2c40522..a12e7ff 100644 --- a/tests/test_corpus.py +++ b/tests/test_corpus.py @@ -287,6 +287,82 @@ def test_linked_movie_export_oracle_checks_all_xml_and_movie_bytes(self): with self.assertRaises(ValueError): corpus.verify_linked_movie_export_xml(original, exported, case) + def test_movie_clip_oracle_preserves_excluded_marks_timing_and_complete_movie(self): + case = next(c for c in corpus.load_manifest()["cases"] if c["id"] == "linked-movie") + original = corpus.local_fixture_path(case) + media = self.root / "média & movie" / "constant-rate.avi" + media.parent.mkdir() + payload = corpus.local_fixture_path(case["assets"][0]).read_bytes() + media.write_bytes(payload) + output = self.root / "clip.trk" + for single in [False, True]: + mode = "single" if single else "stride" + expected, clip_case = corpus.linked_media_clip_expected(case, single) + self.assertEqual([sample["frame"] for sample in expected["point_masses"][0]["samples"]], [0, 1, 2]) + self.assertEqual(expected["clip"]["first_frame"], 2 if single else 1) + self.assertEqual(clip_case["session_media"]["frame_count"], 4) + with corpus.minidom.parse(str(original)) as document: + corpus.patch_expected_linked_media(document, movie=True, clip_mode=mode) + correct = document.toxml() + output.write_text(correct, encoding="utf-8") + corpus.verify_linked_movie_export_xml(original, output, case, mode) + for name, value in [("startframe", "0"), ("stepsize", "3"), ("stepcount", "4"), ("starttime", "0")]: + root = ET.fromstring(correct) + root.find(f"property[@name='videoclip']/object/property[@name='{name}']").text = value + ET.ElementTree(root).write(output, encoding="utf-8") + with self.subTest(mode=mode, field=name), self.assertRaises(ValueError): + corpus.verify_linked_movie_export_xml(original, output, case, mode) + output.write_text(correct.replace("{0,1,2}", "{1,2}"), encoding="utf-8") + with self.assertRaises(ValueError): + corpus.verify_linked_movie_export_xml(original, output, case, mode) + output.write_text(correct, encoding="utf-8") + media.write_bytes(payload[::-1]) + with self.assertRaises(ValueError): + corpus.verify_linked_movie_export_xml(original, output, case, mode) + media.write_bytes(payload) + + def test_saved_movie_oracle_checks_reference_and_all_bytes_after_clipped_resave(self): + case = next(c for c in corpus.load_manifest()["cases"] if c["id"] == "linked-movie") + asset = case["assets"][0] + movie = self.root / "média & movie" / "constant-rate.avi" + movie.parent.mkdir() + payload = corpus.local_fixture_path(asset).read_bytes() + movie.write_bytes(payload) + saved = self.root / "tracker-resaved.trk" + with corpus.minidom.parse(str(corpus.local_fixture_path(case))) as document: + corpus.patch_expected_linked_media(document, movie=True, clip_mode="single") + valid = document.toxml() + for reference in ["média & movie/constant-rate.avi", str(movie), movie.as_uri()]: + root = ET.fromstring(valid) + root.find(".//property[@name='path']").text = reference + ET.ElementTree(root).write(saved, encoding="utf-8") + observed = corpus.verify_saved_movie_asset(saved, asset) + self.assertEqual(observed, {"reference": reference, "bytes": asset["bytes"], "sha256": asset["sha256"]}) + for mutation in ["network", "network-file", "network-share", "archive", "missing", "duplicate", "type", "class", "nested"]: + root = ET.fromstring(valid) + video = root.find("property[@name='videoclip']/object/property[@name='video']/object") + path = video.find("property[@name='path']") + if mutation == "network": path.text = "https://example.invalid/movie.avi" + elif mutation == "network-file": path.text = "file://example.invalid/movie.avi" + elif mutation == "network-share": path.text = "//example.invalid/movie.avi" + elif mutation == "archive": path.text = "archive.trz!/movie.avi" + elif mutation == "missing": video.remove(path) + elif mutation == "duplicate": video.append(copy.deepcopy(path)) + elif mutation == "type": path.set("type", "object") + elif mutation == "class": video.set("class", "UnknownVideo") + else: ET.SubElement(path, "object") + ET.ElementTree(root).write(saved, encoding="utf-8") + with self.subTest(mutation=mutation), self.assertRaises(ValueError): + corpus.verify_saved_movie_asset(saved, asset) + saved.write_text(valid, encoding="utf-8") + for corrupted in [payload[:-1], payload[::-1], payload + b"extra"]: + movie.write_bytes(corrupted) + with self.assertRaisesRegex(ValueError, "checksum"): + corpus.verify_saved_movie_asset(saved, asset) + movie.unlink() + with self.assertRaises(OSError): + corpus.verify_saved_movie_asset(saved, asset) + def test_linked_image_export_oracle_checks_edited_geometry_all_xml_and_relocated_assets(self): manifest = corpus.load_manifest() case = next(c for c in manifest["cases"] if c["id"] == "linked-images") @@ -338,7 +414,7 @@ def test_image_clip_oracle_checks_range_all_marks_and_excluded_image_bytes(self) output = self.root / "clip.trk" for single in [False, True]: mode = "single" if single else "stride" - expected, case = corpus.linked_image_clip_expected(original_case, single) + expected, case = corpus.linked_media_clip_expected(original_case, single) self.assertEqual(expected["clip"], {"first_frame": 2 if single else 1, "step_size": 1 if single else 2, "step_count": 1 if single else 2, "start_ms": -250.0}) self.assertEqual([sample["frame"] for sample in expected["point_masses"][0]["samples"]], [0, 1, 2]) @@ -370,7 +446,7 @@ def test_image_clip_oracle_checks_range_all_marks_and_excluded_image_bytes(self) excluded.write_bytes(image) self.assertEqual(original_case["session_media"]["frames"][0]["frame"], 0) with self.assertRaisesRegex(ValueError, "pinned"): - corpus.linked_image_clip_expected({**original_case, "id": "unrelated"}) + corpus.linked_media_clip_expected({**original_case, "id": "unrelated"}) with corpus.minidom.parse(str(original)) as document, self.assertRaisesRegex(ValueError, "Unknown"): corpus.patch_expected_linked_media(document, clip_mode="unreviewed") @@ -454,10 +530,11 @@ def test_saved_image_inventory_resolves_local_and_archive_references_after_resav with self.assertRaisesRegex(ValueError, "checksum.*frame 0"): corpus.verify_saved_image_inventory(saved, assets) - def test_image_clip_session_checks_strided_and_single_frame_bounds(self): - for mode in ["stride", "single"]: - with self.subTest(mode=mode): - self.check_media_session_oracle("linked-images", mode) + def test_media_clip_session_checks_strided_and_single_frame_bounds(self): + for case_id in ["linked-images", "linked-movie"]: + for mode in ["stride", "single"]: + with self.subTest(case_id=case_id, mode=mode): + self.check_media_session_oracle(case_id, mode) def test_media_session_requires_independent_pixels_order_and_timing(self): for case_id in ["linked-images", "linked-movie"]: @@ -469,7 +546,7 @@ def check_media_session_oracle(self, case_id, clip_mode=None): case = next(c for c in manifest["cases"] if c["id"] == case_id) expected = json.loads(corpus.expected_path(case).read_text())["project"] if clip_mode is not None: - expected, case = corpus.linked_image_clip_expected(case, clip_mode == "single") + expected, case = corpus.linked_media_clip_expected(case, clip_mode == "single") keys = [[0, 1, 2]] if clip_mode is not None else None mass = expected["point_masses"][0] before = {"tracks": [case["session_tracks"][0], { diff --git a/tests/tracker_writer_test.cpp b/tests/tracker_writer_test.cpp index 35182b1..e5bb2e4 100644 --- a/tests/tracker_writer_test.cpp +++ b/tests/tracker_writer_test.cpp @@ -126,6 +126,50 @@ TEST(TrackerWriter, SavesLinkedMovieEditsAfterNativeReopenSourceRemovalAndReloca EXPECT_NO_THROW(motrack::writeTrackerDocument(output, reopened, output)); } +TEST(TrackerWriter, SavesStridedAndSingleFrameMovieClipsAfterNativeReopenWithoutDroppingMarks) +{ + for (const bool single : {false, true}) + { + SCOPED_TRACE(single); + QTemporaryDir temporary; + const auto source = QString::fromUtf8(MOTRACK_FIXTURES) + "/../corpus/fixtures/linked-movie/linked-movie.trk"; + auto document = indexedMovieDocument(source); + const auto sourceBytes = retainedSource(document).xml; + const auto movie = QFileInfo(source).absolutePath() + "/constant-rate.avi"; + const auto movieBytes = readBytes(movie); + motrack::TrackEdits edits; + ASSERT_TRUE(edits.markPointMass(document.project, 0, {.frame = 1, .x = 8.0, .y = 12.0})); + const auto before = document.project; + ASSERT_TRUE(edits.setClipRange(document.project, single ? 2 : 1, single ? 2 : 3, single ? 1 : 2)); + const auto clipped = document.project; + ASSERT_TRUE(edits.undo(document.project)); + EXPECT_EQ(document.project, before); + ASSERT_TRUE(edits.redo(document.project)); + EXPECT_EQ(document.project, clipped); + const auto native = temporary.filePath("clip.motrack"); + motrack::rebaseWorkspaceMedia(document, source, native); + motrack::writeWorkspaceDocument(native, document); + document = motrack::readWorkspaceDocument(native); + const auto target = temporary.filePath("clip.trk"); + ASSERT_NO_THROW(motrack::writeTrackerDocument(target, document, native)); + const auto reopened = motrack::importTrackerDocument(target).project; + EXPECT_EQ(reopened.clip.firstFrame, single ? 2 : 1); + EXPECT_EQ(reopened.clip.stepSize, single ? 1 : 2); + EXPECT_EQ(reopened.clip.stepCount, single ? 1 : 2); + EXPECT_EQ(reopened.pointMasses, before.pointMasses); + EXPECT_DOUBLE_EQ(reopened.clip.startTimeMs, -250.0); + EXPECT_EQ(document.movieFrameTimes, (std::vector{0.0, 0.125, 0.25, 0.375})); + EXPECT_EQ(document.project.clip.sourceFrameCount, 4); + EXPECT_EQ(retainedSource(document).xml, sourceBytes); + EXPECT_EQ(readBytes(movie), movieBytes); + auto expanded = indexedMovieDocument(target); + ASSERT_TRUE(edits.setClipRange(expanded.project, 0, 3, 1)); + ASSERT_NO_THROW(motrack::writeTrackerDocument(target, expanded, target)); + EXPECT_EQ(motrack::importTrackerDocument(target).project.pointMasses, before.pointMasses); + EXPECT_EQ(motrack::importTrackerDocument(target).project.clip.stepCount, 4); + } +} + TEST(TrackerWriter, SavesMovieDefaultNumberFormatsAndRejectsCustomFormatsAfterNativeReopen) { for (const bool custom : {false, true}) @@ -204,7 +248,10 @@ TEST(TrackerWriter, RejectsUnsupportedMovieStateBeforeReplacingExistingOutput) [](auto& doc) { doc.project.clip.sourceFrameCount = 3; }, [](auto& doc) { doc.project.clip.startTimeMs = 0.0; }, [](auto& doc) { doc.project.clip.frameDurationMs = 125.0; }, - [](auto& doc) { doc.project.clip.stepCount = 3; }, + [](auto& doc) { doc.project.clip.stepCount = 5; }, + [](auto& doc) { doc.project.clip.stepSize = 0; }, + [](auto& doc) { doc.project.clip.firstFrame = 4; }, + [](auto& doc) { doc.project.clip.stepCount = 0; }, [](auto& doc) { doc.project.pointMasses.front().name = "renamed"; }, [](auto& doc) { doc.project.pointMasses.front().marks.front().frame = 4; }, [](auto& doc) { doc.project.pointMasses.clear(); }, diff --git a/tools/corpus.py b/tools/corpus.py index c70ce70..4842e46 100644 --- a/tools/corpus.py +++ b/tools/corpus.py @@ -474,9 +474,9 @@ def linked_media_roundtrip_expected(case): return expected -def linked_image_clip_expected(case, single=False): - if case["id"] not in {"linked-images", "archived-images"}: - raise ValueError("Image clip edits require a pinned linked/archived-images case") +def linked_media_clip_expected(case, single=False): + if case["id"] not in {"linked-images", "archived-images", "linked-movie"}: + raise ValueError("Media clip edits require a pinned image/movie case") expected = linked_media_roundtrip_expected(case) expected["clip"].update(first_frame=2 if single else 1, step_size=1 if single else 2, step_count=1 if single else 2) for sample, frame in zip(expected["coordinate_samples"], [2, 2] if single else [1, 3]): @@ -492,7 +492,7 @@ def linked_image_clip_expected(case, single=False): def patch_expected_linked_media(document, relocate=True, packaged=False, movie=False, clip_mode=None): if clip_mode not in {None, "stride", "single"}: - raise ValueError("Unknown linked-image clip mode") + raise ValueError("Unknown linked-media clip mode") def elements(node, tag): return [child for child in node.childNodes if child.nodeType == child.ELEMENT_NODE and child.tagName == tag] def prop(node, name): @@ -607,11 +607,43 @@ def verify_saved_image_inventory(saved, assets): return inventory -def verify_linked_movie_export_xml(original, exported, case): +def verify_saved_movie_asset(saved, asset): + """Verify the complete pinned movie after Tracker saves, including excluded frames.""" + if saved.stat().st_size > MAX_XML_BYTES: + raise ValueError("Saved movie experiment exceeds XML limit") + videos = ET.parse(saved).getroot().findall("property[@name='videoclip']/object/property[@name='video']/object") + if len(videos) != 1 or videos[0].get("class") != "org.opensourcephysics.media.xuggle.XuggleVideo": + raise ValueError("Saved movie requires exactly one XuggleVideo") + paths = videos[0].findall("property[@name='path']") + if (len(paths) != 1 or paths[0].get("type") != "string" or len(paths[0]) + or not paths[0].text or len(paths[0].text) > 4096): + raise ValueError("Saved movie lost its local path") + reference = paths[0].text.replace("\\", "/") + url = urllib.parse.urlsplit(reference) + if url.scheme == "file": + if url.netloc not in {"", "localhost"} or url.query or url.fragment: + raise ValueError("Saved movie reference is not a local file") + reference = urllib.request.url2pathname(url.path) + elif url.scheme and not re.match(r"^[A-Za-z]:/", reference): + raise ValueError("Saved movie reference is not a local file") + if reference.replace("\\", "/").startswith("//") or "!/" in reference: + raise ValueError("Saved movie reference is not a standalone local file") + path = Path(reference) + if not path.is_absolute(): + path = saved.parent / path + with path.open("rb") as source: + payload = source.read(asset["bytes"] + 1) + digest = hashlib.sha256(payload).hexdigest() + if len(payload) != asset["bytes"] or digest != asset["sha256"]: + raise ValueError("Saved movie checksum changed") + return {"reference": paths[0].text, "bytes": len(payload), "sha256": digest} + + +def verify_linked_movie_export_xml(original, exported, case, clip_mode=None): with minidom.parse(str(original)) as expected, minidom.parse(str(exported)) as actual: - patch_expected_linked_media(expected, movie=True) + patch_expected_linked_media(expected, movie=True, clip_mode=clip_mode) if canonical_tracker_xml(expected) != canonical_tracker_xml(actual): - raise ValueError("Linked-movie export changed content beyond the requested path, mass and mark") + raise ValueError("Linked-movie export changed content beyond the requested path, mass, mark and clip range") verify_file(exported.parent / "média & movie" / "constant-rate.avi", case["assets"][0]) @@ -1893,7 +1925,10 @@ def execute(input_file, destination, stage, metadata_args=()): def check_sessions(manifest, cache, java, probe=None): if sys.platform != "linux": raise ValueError("Session checks currently require Linux with Xvfb") - modes = [("linked-movie", "native_linked_movie_roundtrip"), ("archive-images", "native_archive_image_roundtrip"), ("linked-images", "native_linked_image_roundtrip"), + modes = [("linked-movie", "native_linked_movie_roundtrip"), + ("linked-movie-clip", "native_linked_movie_clip_roundtrip"), + ("linked-movie-clip-single", "native_linked_movie_clip_roundtrip"), + ("archive-images", "native_archive_image_roundtrip"), ("linked-images", "native_linked_image_roundtrip"), ("linked-image-clip", "native_linked_image_clip_roundtrip"), ("linked-image-clip-single", "native_linked_image_clip_roundtrip"), ("archive-image-clip", "native_archive_image_clip_roundtrip"), ("packaged-image-clip", "native_linked_image_clip_roundtrip"), ("packaged-images", "native_image_package_roundtrip"), @@ -1996,7 +2031,7 @@ def check_sessions(manifest, cache, java, probe=None): project = selected expected = linked_media_roundtrip_expected(case) if mode == "packaged-image-clip": - expected, case = linked_image_clip_expected(case, single=True) + expected, case = linked_media_clip_expected(case, single=True) native_report = output / "native-export.json" run([str(Path(probe).resolve()), str(project), str(native_report)]) compare(expected, json.loads(native_report.read_text(encoding="utf-8")), manifest["tolerance"], case["id"] + ".export") @@ -2008,7 +2043,7 @@ def check_sessions(manifest, cache, java, probe=None): verify_archive_image_export(archive_input, exported, project, clip_mode="stride" if mode == "archive-image-clip" else None) expected = linked_media_roundtrip_expected(case) if mode == "archive-image-clip": - expected, case = linked_image_clip_expected(case) + expected, case = linked_media_clip_expected(case) native_report = output / "native-export.json" run([str(Path(probe).resolve()), str(project), str(native_report)]) compare(expected, json.loads(native_report.read_text(encoding="utf-8")), manifest["tolerance"], case["id"] + ".export") @@ -2030,13 +2065,17 @@ def check_sessions(manifest, cache, java, probe=None): if mode == "linked-movie": verify_linked_movie_export_xml(project, exported, case) expected = linked_media_roundtrip_expected(case) + elif mode in {"linked-movie-clip", "linked-movie-clip-single"}: + single = mode == "linked-movie-clip-single" + verify_linked_movie_export_xml(project, exported, case, "single" if single else "stride") + expected, case = linked_media_clip_expected(case, single) elif mode in {"linked-images", "paired-linked-images"}: verify_linked_image_export_xml(project, exported, case) expected = linked_media_roundtrip_expected(case) elif mode in {"linked-image-clip", "linked-image-clip-single"}: single = mode == "linked-image-clip-single" verify_linked_image_export_xml(project, exported, case, "single" if single else "stride") - expected, case = linked_image_clip_expected(case, single) + expected, case = linked_media_clip_expected(case, single) elif mode == "mass": verify_mass_export_xml(project, exported) expected = mass_roundtrip_expected(case) @@ -2097,6 +2136,7 @@ def check_sessions(manifest, cache, java, probe=None): project = exported stages = {} image_inventories = {} + movie_inventories = {} # Tracker has static per-panel drawing state but allocates IDs separately in each TFrame. # A fresh JVM prevents two windows' vector snapping registries from sharing the same IDs. for stage in ["save", "reopen"]: @@ -2128,13 +2168,15 @@ def check_sessions(manifest, cache, java, probe=None): # Both cases use the independently pinned four-image fixture, including excluded frames. assets = next(item["assets"] for item in manifest["cases"] if item["id"] == "linked-images") image_inventories[stage] = verify_saved_image_inventory(output / "tracker-resaved.trk", assets) + elif case["id"] == "linked-movie": + movie_inventories[stage] = verify_saved_movie_asset(output / "tracker-resaved.trk", case["assets"][0]) report = {"reference_version": stages["save"]["reference_version"], "verification_level": "tracker_session_resave", "view_preparation": stages["save"]["view_preparation"], "loader_callbacks": {stage: stages[stage]["loader_callbacks"] for stage in stages}, "before": stages["save"]["loaded"], "after_save": stages["save"]["after_save"], - "after": stages["reopen"]["loaded"], "saved_image_inventories": image_inventories} + "after": stages["reopen"]["loaded"], "saved_image_inventories": image_inventories, "saved_movie_assets": movie_inventories} keys = {"marks": [[0, 3], [7]], "empty-marks": [[], [0]], "first-mark": [[4], [0]]}.get(mode) - if mode in {"linked-movie", "linked-images", "linked-image-clip", "linked-image-clip-single", "archive-images", + if mode in {"linked-movie", "linked-movie-clip", "linked-movie-clip-single", "linked-images", "linked-image-clip", "linked-image-clip-single", "archive-images", "paired-linked-images", "paired-archive-images", "packaged-images", "archive-image-clip", "packaged-image-clip"}: keys = [[0, 1, 2]] if mode in {"create-mass", "create-empty-mass"}: From 4ffccaf72b4011cfe0553f0161f30ea390263d0c Mon Sep 17 00:00:00 2001 From: Matt Gradwohl <3945134+mgradwohl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:20:57 -0600 Subject: [PATCH 2/5] Restore complete movie bounds after indexing a reopened clip --- docs/native-format.md | 8 +++++ src/main_window.cpp | 2 ++ tests/paired_workspace_test.cpp | 58 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/docs/native-format.md b/docs/native-format.md index a153186..9ff9ac9 100644 --- a/docs/native-format.md +++ b/docs/native-format.md @@ -699,3 +699,11 @@ groups (124.80 seconds), 86 Python tests, both native export/oracle/reimport cas static analysis for all four changed C++ source files, with 95.64% line coverage (8838/9241). Three existing LLVM function-profile mismatch warnings remain visible. Current-head three-platform CI, actual Tracker movie-range acceptance and review remain required before merging this extension. + + +A standard movie import initially infers a provisional source-frame bound from its saved clip endpoint. +When the linked movie's full index completes, the desktop replaces that bound with the decoded source +count. Otherwise a reopened single-frame clip could not expand to retained frames beyond its endpoint. +A desktop regression reproduced that failure, then verified single-frame export, standard-file reopen, +index completion, expansion to all four frames, paired resave and display of the retained final frame. +Index completion does not mark the document modified or remove measurements. diff --git a/src/main_window.cpp b/src/main_window.cpp index 7cfe502..05f773f 100644 --- a/src/main_window.cpp +++ b/src/main_window.cpp @@ -955,6 +955,8 @@ void MainWindow::scheduleMovieIndex(const QString& path, bool standalone) if (!index.frameTimes.empty()) { m_Session.document.movieFrameTimes = std::move(index.frameTimes); + // The imported clip endpoint is only a provisional source bound. + m_Session.document.project.clip.sourceFrameCount = index.frameCount; } refreshPlot(); return; diff --git a/tests/paired_workspace_test.cpp b/tests/paired_workspace_test.cpp index fccdb15..e3e2b52 100644 --- a/tests/paired_workspace_test.cpp +++ b/tests/paired_workspace_test.cpp @@ -132,6 +132,64 @@ TEST(PairedWorkspace, SavesIndexedMovieEditsAndNativeCompanionAfterSourceRemoval EXPECT_DOUBLE_EQ(motrack::importTrackerDocument(target).project.pointMasses.front().mass, 9); } +TEST(PairedWorkspace, ReopenedSingleFrameMovieCanExpandToTheCompleteIndexedSource) +{ + QTemporaryDir temporary; + const auto source = QString::fromUtf8(MOTRACK_FIXTURES) + "/../corpus/fixtures/linked-movie/linked-movie.trk"; + const auto target = temporary.filePath("single.trk"); + motrack::MainWindow window; + ASSERT_TRUE(window.loadProject(source)); + auto* plot = window.findChild("positionPlot"); + auto* canvas = window.findChild("videoCanvas"); + auto* slider = window.findChild("stepSlider"); + auto* settings = window.findChild("clipSettings"); + ASSERT_TRUE(plot && canvas && slider && settings); + for (const bool expand : {false, true}) + { + SCOPED_TRACE(expand); + ASSERT_TRUE(QTest::qWaitFor([plot] { return plot->accessibleName().endsWith("Time (s)"); })); + EXPECT_FALSE(window.isWindowModified()); + int maximum = -1; + bool accepted = false; + QTimer::singleShot(0, + &window, + [&] + { + if (auto* dialog = qobject_cast(QApplication::activeModalWidget())) + { + auto* first = dialog->findChild("clipStart"); + auto* last = dialog->findChild("clipEnd"); + auto* stride = dialog->findChild("clipStride"); + if (!first || !last || !stride) + { + dialog->reject(); + return; + } + maximum = last->maximum(); + first->setValue(expand ? 0 : 2); + last->setValue(expand ? 3 : 2); + stride->setValue(1); + accepted = true; + dialog->accept(); + } + }); + settings->trigger(); + ASSERT_TRUE(accepted); + ASSERT_EQ(maximum, 3); + ASSERT_TRUE(window.saveTrackerProject(target)); + const auto saved = motrack::readWorkspaceDocument(temporary.filePath("single.motrack")); + EXPECT_EQ(saved.project.clip.sourceFrameCount, 4); + EXPECT_EQ(saved.project.clip.firstFrame, expand ? 0 : 2); + EXPECT_EQ(saved.project.clip.stepCount, expand ? 4 : 1); + EXPECT_EQ(saved.project.pointMasses, motrack::importTrackerDocument(source).project.pointMasses); + if (!expand) + ASSERT_TRUE(window.loadProject(target)); + } + slider->setValue(3); + ASSERT_TRUE(QTest::qWaitFor([canvas] { return canvas->accessibleName() == "Frame 3"; })); + EXPECT_EQ(motrack::importTrackerDocument(target).project.clip.stepCount, 4); +} + TEST(PairedWorkspace, SavesEditedLinkedAndArchivedProjectsAndUsesThePairForOrdinarySave) { for (const bool archive : {false, true}) From d230ae79fc8602bc03f58deb444b82f47ae2d3a9 Mon Sep 17 00:00:00 2001 From: Matt Gradwohl <3945134+mgradwohl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:25:51 -0600 Subject: [PATCH 3/5] Record validation of the movie reopen correction --- docs/native-format.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/native-format.md b/docs/native-format.md index 9ff9ac9..cad5d52 100644 --- a/docs/native-format.md +++ b/docs/native-format.md @@ -694,7 +694,7 @@ The movie-range extension adds strided and single-frame native export/reopen cas Settings/undo/redo/paired-save checks and two actual-Tracker acceptance modes. Independent whole-XML expectations permit only the requested range, mass, mark and path; every mark outside the clip remains. After Tracker saves and freshly reopens, the saved movie reference must resolve locally to the complete -pinned byte count/SHA-256, including excluded frames. Local Windows validation passes all nine CTest +pinned byte count/SHA-256, including excluded frames. Initial Windows validation passes all nine CTest groups (124.80 seconds), 86 Python tests, both native export/oracle/reimport cases, formatting and static analysis for all four changed C++ source files, with 95.64% line coverage (8838/9241). Three existing LLVM function-profile mismatch warnings remain visible. Current-head three-platform CI, @@ -707,3 +707,9 @@ count. Otherwise a reopened single-frame clip could not expand to retained frame A desktop regression reproduced that failure, then verified single-frame export, standard-file reopen, index completion, expansion to all four frames, paired resave and display of the retained final frame. Index completion does not mark the document modified or remove measurements. + +After the reopen-bound correction, all nine Windows CTest groups pass in 132.22 seconds with 95.64% +production line coverage (8839/9242). The new desktop regression first failed at the shortened bound +and then passed with the correction; all three related desktop cases, both affected source-file static +analyses and formatting pass. Three existing LLVM profile-mismatch warnings remain visible. Actual +Tracker and three-platform acceptance still require the final published revision's CI. From b56cd4efcfea2de86c0ce6e4d30c46699bdd1167 Mon Sep 17 00:00:00 2001 From: Matt Gradwohl <3945134+mgradwohl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:51:50 -0600 Subject: [PATCH 4/5] Keep checked image sequence outside the test frame loop --- docs/native-format.md | 6 ++++-- tests/paired_workspace_test.cpp | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/native-format.md b/docs/native-format.md index cad5d52..1f791fa 100644 --- a/docs/native-format.md +++ b/docs/native-format.md @@ -708,8 +708,10 @@ A desktop regression reproduced that failure, then verified single-frame export, index completion, expansion to all four frames, paired resave and display of the retained final frame. Index completion does not mark the document modified or remove measurements. -After the reopen-bound correction, all nine Windows CTest groups pass in 132.22 seconds with 95.64% -production line coverage (8839/9242). The new desktop regression first failed at the shortened bound +After the reopen-bound correction, Linux static analysis required a checked image-sequence reference +outside the test's frame loop. With that guard and the merged journal-group split, all ten Windows +CTest groups pass in 150.73 seconds with 95.65% production line coverage (8840/9242). +The new desktop regression first failed at the shortened bound and then passed with the correction; all three related desktop cases, both affected source-file static analyses and formatting pass. Three existing LLVM profile-mismatch warnings remain visible. Actual Tracker and three-platform acceptance still require the final published revision's CI. diff --git a/tests/paired_workspace_test.cpp b/tests/paired_workspace_test.cpp index e3e2b52..937e775 100644 --- a/tests/paired_workspace_test.cpp +++ b/tests/paired_workspace_test.cpp @@ -325,9 +325,10 @@ TEST(PairedWorkspace, SavesMediaClipSettingsAndReopensTheSameSourceFramesThrough const auto sequence = motrack::ImageSequence::open(native, reopened.project.clip); if (!sequence) FAIL() << "The saved clip lost its image sequence."; + const auto& savedSequence = sequence.value(); for (int step = 0; step < 2; ++step) { - const auto frame = sequence->readStep(step); + const auto frame = savedSequence.readStep(step); if (!frame) FAIL() << "The saved clip lost a source frame."; EXPECT_EQ(frame->image, QImage(fixture("images/frame" + QString::number(1 + step * 2) + ".png"))); From 4cf2b0f2e125c52518571334bc7d1138c51bfc43 Mon Sep 17 00:00:00 2001 From: Matt Gradwohl <3945134+mgradwohl@users.noreply.github.com> Date: Sun, 8 Nov 2026 22:19:19 -0600 Subject: [PATCH 5/5] Preserve recorded clip bounds when linked movies become shorter --- docs/native-format.md | 20 +++++++++---- src/main_window.cpp | 10 +++++-- tests/paired_workspace_test.cpp | 50 +++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/docs/native-format.md b/docs/native-format.md index 1f791fa..3421504 100644 --- a/docs/native-format.md +++ b/docs/native-format.md @@ -702,16 +702,24 @@ actual Tracker movie-range acceptance and review remain required before merging A standard movie import initially infers a provisional source-frame bound from its saved clip endpoint. -When the linked movie's full index completes, the desktop replaces that bound with the decoded source -count. Otherwise a reopened single-frame clip could not expand to retained frames beyond its endpoint. +When the linked movie's full index completes and contains the saved clip, the desktop extends that +bound to the decoded source count without reducing a previously known bound. Otherwise a reopened +single-frame clip could not expand to retained frames beyond its endpoint. A desktop regression reproduced that failure, then verified single-frame export, standard-file reopen, index completion, expansion to all four frames, paired resave and display of the retained final frame. Index completion does not mark the document modified or remove measurements. +If a replacement movie is shorter than the recorded clip, indexing retains the original bound rather +than producing an invalid native document. A regression records a five-frame clip and a mark on frame +4 with an available four-frame movie: native save/reopen retains the range, mark and available index. +An already trimmed native copy also retains its larger known bound, so indexing cannot invalidate an +earlier range that can be restored through undo. +Standard export still rejects this incomplete media/index match and preserves the existing destination. + After the reopen-bound correction, Linux static analysis required a checked image-sequence reference -outside the test's frame loop. With that guard and the merged journal-group split, all ten Windows -CTest groups pass in 150.73 seconds with 95.65% production line coverage (8840/9242). -The new desktop regression first failed at the shortened bound -and then passed with the correction; all three related desktop cases, both affected source-file static +outside the test's frame loop. With that guard, retained movie bounds and the merged journal-group split, +all ten Windows CTest groups pass in 129.56 seconds with 95.64% production line coverage (8844/9247). +Desktop regressions reproduced both the shortened expansion bound and an unreadable native save +after indexing shorter media, then passed with the corrections. All four related desktop cases, both affected source-file static analyses and formatting pass. Three existing LLVM profile-mismatch warnings remain visible. Actual Tracker and three-platform acceptance still require the final published revision's CI. diff --git a/src/main_window.cpp b/src/main_window.cpp index 05f773f..405b541 100644 --- a/src/main_window.cpp +++ b/src/main_window.cpp @@ -955,8 +955,14 @@ void MainWindow::scheduleMovieIndex(const QString& path, bool standalone) if (!index.frameTimes.empty()) { m_Session.document.movieFrameTimes = std::move(index.frameTimes); - // The imported clip endpoint is only a provisional source bound. - m_Session.document.project.clip.sourceFrameCount = index.frameCount; + auto& clip = m_Session.document.project.clip; + const auto steps = clip.stepCount.value_or(0); + const auto last = + static_cast(clip.firstFrame) + static_cast(steps - 1) * clip.stepSize; + // Extend the provisional bound only if the movie contains the saved + // clip. Retain known bounds for earlier ranges and their undo history. + if (steps == 0 || last < index.frameCount) + clip.sourceFrameCount = std::max(clip.sourceFrameCount.value_or(0), index.frameCount); } refreshPlot(); return; diff --git a/tests/paired_workspace_test.cpp b/tests/paired_workspace_test.cpp index 937e775..e4ffab4 100644 --- a/tests/paired_workspace_test.cpp +++ b/tests/paired_workspace_test.cpp @@ -190,6 +190,56 @@ TEST(PairedWorkspace, ReopenedSingleFrameMovieCanExpandToTheCompleteIndexedSourc EXPECT_EQ(motrack::importTrackerDocument(target).project.clip.stepCount, 4); } +TEST(PairedWorkspace, ShorterLinkedMovieKeepsSavedClipAndMeasurementsReadable) +{ + QTemporaryDir temporary; + const auto movieFixture = QString::fromUtf8(MOTRACK_FIXTURES) + "/../corpus/fixtures/linked-movie/"; + const auto source = temporary.filePath("longer-clip.trk"); + const auto native = temporary.filePath("retained.motrack"); + // The recorded clip spans five frames, but the available replacement movie has only four. + auto xml = bytes(movieFixture + "linked-movie.trk"); + xml.replace(R"(name="stepcount" type="int">4<)", R"(name="stepcount" type="int">5<)"); + xml.replace(R"(name="[2]" type="object")", R"(name="[4]" type="object")"); + put(source, xml); + ASSERT_TRUE(QFile::copy(movieFixture + "constant-rate.avi", temporary.filePath("constant-rate.avi"))); + const auto original = motrack::importTrackerDocument(source).project; + ASSERT_EQ(original.clip.stepCount, 5); + ASSERT_EQ(original.pointMasses.at(0).marks.back().frame, 4); + motrack::MainWindow window; + ASSERT_TRUE(window.loadProject(source)); + auto* plot = window.findChild("positionPlot"); + auto* slider = window.findChild("stepSlider"); + ASSERT_TRUE(plot && slider); + ASSERT_TRUE(QTest::qWaitFor([plot] { return plot->accessibleName().endsWith("Time (s)"); })); + EXPECT_FALSE(window.isWindowModified()); + ASSERT_TRUE(window.saveWorkspace(native)); + const auto saved = motrack::readWorkspaceDocument(native); + EXPECT_EQ(saved.project.clip.sourceFrameCount, 5); + EXPECT_EQ(saved.project.clip.firstFrame, 0); + EXPECT_EQ(saved.project.clip.stepSize, 1); + EXPECT_EQ(saved.project.clip.stepCount, 5); + EXPECT_EQ(saved.project.pointMasses, original.pointMasses); + EXPECT_EQ(saved.movieFrameTimes, (std::vector{0.0, 0.125, 0.25, 0.375})); + const auto standard = temporary.filePath("unchanged.trk"); + put(standard, "existing output"); + EXPECT_FALSE(window.saveTrackerProject(standard)); + EXPECT_EQ(bytes(standard), "existing output"); + ASSERT_TRUE(window.loadProject(native)); + EXPECT_EQ(slider->maximum(), 4); + EXPECT_FALSE(window.isWindowModified()); + auto trimmed = saved; + trimmed.project.clip.stepCount = 3; + trimmed.movieFrameTimes.clear(); + const auto trimmedPath = temporary.filePath("trimmed.motrack"); + motrack::writeWorkspaceDocument(trimmedPath, trimmed); + ASSERT_TRUE(window.loadProject(trimmedPath)); + ASSERT_TRUE(QTest::qWaitFor([plot] { return plot->accessibleName().endsWith("Time (s)"); })); + ASSERT_TRUE(window.saveWorkspace(trimmedPath)); + const auto retrimmed = motrack::readWorkspaceDocument(trimmedPath); + EXPECT_EQ(retrimmed.project.clip.sourceFrameCount, 5); + EXPECT_EQ(retrimmed.project.pointMasses, original.pointMasses); +} + TEST(PairedWorkspace, SavesEditedLinkedAndArchivedProjectsAndUsesThePairForOrdinarySave) { for (const bool archive : {false, true})