Adopt Shotcut's melt invocation shape; show version in the nav drawer #9

Merged
cheapnud merged 1 commit from d/melt-invocation into master 2026-08-10 19:11:46 +00:00
Owner

Every render now goes through a render-only XML in system temp with absolute media paths and the avformat consumer embedded as an XML element, invoked as melt -verbose -progress2 -abort xml:, with the multi consumer engaged when resolution or frame rate overrides the project profile. Smoke-tested against the installed melt including an ffprobe-verified 720p30 override render. Also shows the app version at the bottom of the nav drawer like the sibling apps.

Every render now goes through a render-only XML in system temp with absolute media paths and the avformat consumer embedded as an XML element, invoked as melt -verbose -progress2 -abort xml:<percent-encoded path>, with the multi consumer engaged when resolution or frame rate overrides the project profile. Smoke-tested against the installed melt including an ffprobe-verified 720p30 override render. Also shows the app version at the bottom of the nav drawer like the sibling apps.
adopt shotcut's melt invocation: embedded consumer element, percent-encoded xml producer url, multi consumer for profile overrides, absolute-path render xml; show app version in nav drawer
All checks were successful
Claude PR Review / AI Code Review (pull_request) Successful in 20s
Build & Test / Build and Test (pull_request) Successful in 26s
693752e36a

🤖 Claude AI Code Review

  1. Summary
    Refactors melt invocation to Shotcut's XML-consumer shape and adds an app version display in the nav drawer. Well-organized and documented change.

  2. Code Quality

    Clear separation of concerns and excellent comments explaining the Shotcut-parity rationale.

  3. Issues

🟡 MeltRenderService.cs — Uri.EscapeDataString may over-escape the path for xml: URL. EscapeDataString percent-encodes /, \, and : (drive-letter colon on Windows), which melt's xml: producer parser may not decode back to a valid filesystem path. Shotcut uses QUrl::toPercentEncoding with reserved chars excluded (it does not encode path separators). Verify melt actually opens the resulting URL on Windows paths like C:\Temp\... — the smoke test was on a single platform. Consider escaping only problematic chars (&, #, spaces) rather than the whole path.

🟡 MeltRenderService.cs — ?multi:1 appended to the percent-encoded URL. Since the whole path was escaped, any ? in the path would already be encoded, but appending the query after Uri.EscapeDataString(renderXmlPath) means the URL is xml:<fully-escaped-path>?multi:1. If melt treats everything after xml: as the file argument (not a real URL), the ?multi:1 suffix may be interpreted as part of the filename and fail. Confirm the multi-consumer path was actually exercised — the smoke test mentions a 720p30 override render, so likely OK, but the path-escaping interaction is worth double-checking.

🟢 MeltRenderService.cs — MakeResourcePathsAbsolute iterates project.Chain and appends project.Producer?.Property. Chains/producers can also carry proxy or alternate resource properties; if a producer has multiple resource entries FirstOrDefault only rewrites one. Likely fine given MLT structure, but verify producers don't nest additional resources needing rewrite.

🟢 AppVersion.cs — Assembly.GetEntryAssembly() can return null in some hosting scenarios (already handled via fallback). Fine.

  1. Verdict: REQUEST CHANGES
  • Verify/fix Uri.EscapeDataString over-escaping of the render XML path (path separators, drive-letter colon) so melt's xml: producer can open it cross-platform; escape only reserved chars if needed.
  • Confirm ?multi:1 is not swallowed into the escaped path argument by melt.

📊 Tokens: 10609 input + 915 output | 💰 Cost: ~$0.0759
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review 1. **Summary** Refactors melt invocation to Shotcut's XML-consumer shape and adds an app version display in the nav drawer. Well-organized and documented change. 2. **Code Quality** ⭐⭐⭐⭐☆ Clear separation of concerns and excellent comments explaining the Shotcut-parity rationale. 3. **Issues** 🟡 MeltRenderService.cs — `Uri.EscapeDataString` may over-escape the path for `xml:` URL. `EscapeDataString` percent-encodes `/`, `\`, and `:` (drive-letter colon on Windows), which melt's `xml:` producer parser may not decode back to a valid filesystem path. Shotcut uses `QUrl::toPercentEncoding` with reserved chars excluded (it does *not* encode path separators). Verify melt actually opens the resulting URL on Windows paths like `C:\Temp\...` — the smoke test was on a single platform. Consider escaping only problematic chars (`&`, `#`, spaces) rather than the whole path. 🟡 MeltRenderService.cs — `?multi:1` appended to the percent-encoded URL. Since the whole path was escaped, any `?` in the path would already be encoded, but appending the query after `Uri.EscapeDataString(renderXmlPath)` means the URL is `xml:<fully-escaped-path>?multi:1`. If melt treats everything after `xml:` as the file argument (not a real URL), the `?multi:1` suffix may be interpreted as part of the filename and fail. Confirm the multi-consumer path was actually exercised — the smoke test mentions a 720p30 override render, so likely OK, but the path-escaping interaction is worth double-checking. 🟢 MeltRenderService.cs — `MakeResourcePathsAbsolute` iterates `project.Chain` and appends `project.Producer?.Property`. Chains/producers can also carry proxy or alternate `resource` properties; if a producer has multiple `resource` entries `FirstOrDefault` only rewrites one. Likely fine given MLT structure, but verify producers don't nest additional resources needing rewrite. 🟢 AppVersion.cs — `Assembly.GetEntryAssembly()` can return null in some hosting scenarios (already handled via fallback). Fine. 4. **Verdict**: **REQUEST CHANGES** - Verify/fix `Uri.EscapeDataString` over-escaping of the render XML path (path separators, drive-letter colon) so melt's `xml:` producer can open it cross-platform; escape only reserved chars if needed. - Confirm `?multi:1` is not swallowed into the escaped path argument by melt. --- 📊 **Tokens**: 10609 input + 915 output | 💰 **Cost**: ~$0.0759 *Generated by Claude Opus 4.8 via Gitea Actions*
cheapnud merged commit 809deed20f into master 2026-08-10 19:11:46 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
cheapnud/CheapShotcutRandomizer!9
No description provided.