Cross-platform support: Windows, Linux, and Intel Mac lanes plus loudnorm, captions, and VP9 alpha deliverables - #7
Merged
Merged
Conversation
…m mastering, captions, VP9 alpha deliverable Cross-platform port: onnx-asr transcription lane running the same parakeet-tdt-0.6b-v3 weights (auto provider, PEP 508 dependency markers, chunked long audio, byte-compatible words.json), per-OS venv and torch index in mc-audio, cuda/mps/cpu device ladders, per-OS hardware encoder ladders with one-frame probes, Windows file URI and .cmd shim fixes, UTF-8-explicit artifact IO, and install-time OS/GPU detection with three per-platform stack reference files consumed by mc-setup. Mac-ready enhancements: two-pass loudnorm (-14 LUFS default) at final render, mc-package captions emitter (final.srt, final.vtt, transcript.md from words.json + edl.json), one-command WebM VP9 alpha transcode+verify in mc-stream-pack, free-Resolve Fusion Scripts import lane docs, and the opt-in Resolve MCP pointer. macOS Apple Silicon behavior is unchanged; Windows/Linux lanes are code-complete pending real-hardware validation. All 26 test suites and the genericity gate pass.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe update adds cross-platform setup and transcription lanes, hardware-aware rendering with loudness normalization, edited-timeline caption packaging, alpha WebM verification, portable command execution, and expanded platform and Resolve documentation. ChangesCross-platform production
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant check_deps
participant mc-cut
participant mc-audio
participant mc-package
User->>check_deps: run setup
check_deps->>check_deps: detect platform and GPU
check_deps-->>User: report recommended stack
User->>mc-cut: record media and transcribe
mc-cut->>mc-cut: auto-select provider (Parakeet or ONNX)
mc-cut-->>User: words.json with timings
User->>mc-audio: build audio workspace
mc-audio->>mc-audio: select CUDA or MPS or CPU
mc-audio-->>User: validated venv
User->>mc-package: generate captions
mc-package->>mc-package: project words onto edited timeline
mc-package-->>User: SRT, VTT, transcript.md
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
skills/mc-audio/scripts/farm_audio.py (1)
76-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo helper pairs are hand-duplicated across files, each self-documented as a sync risk.
venv_python()is copy-pasted betweenfarm_audio.pyandensure_workspace.py(not in this batch), andmake_venv_shim()is copy-pasted between the two test files; both docstrings literally say "keep the two in sync," which is a maintenance liability rather than a guarantee.
skills/mc-audio/scripts/farm_audio.py#L76-L88: extractvenv_python()into a small shared module (e.g.skills/mc-audio/scripts/platform_utils.py) imported by bothfarm_audio.pyandensure_workspace.py, removing the duplicate definition here.skills/mc-audio/scripts/tests/test-ensure_workspace.py#L30-L65: import a sharedmake_venv_shim()test helper instead of redefining it, so both test modules exercise the identical fixture logic.skills/mc-audio/scripts/tests/test-farm_audio.py#L29-L92: same — import the sharedmake_venv_shim()helper instead of keeping a second hand-maintained copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/mc-audio/scripts/farm_audio.py` around lines 76 - 88, Remove the duplicated venv_python() definition from skills/mc-audio/scripts/farm_audio.py#L76-L88 and skills/mc-audio/scripts/ensure_workspace.py, move it to a shared platform_utils module, and import it in both callers while preserving OS-specific paths. In skills/mc-audio/scripts/tests/test-ensure_workspace.py#L30-L65 and skills/mc-audio/scripts/tests/test-farm_audio.py#L29-L92, replace each local make_venv_shim() definition with an import from one shared test helper so both suites use identical fixture logic.skills/mc-setup/scripts/check_deps.py (1)
90-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winwmic is disabled by default on current Windows 11 releases; try PowerShell first.
wmichas been off by default since Windows 11 23H2/24H2 and is being fully removed in 2026, so on most current machinesshutil.which("wmic")will already beNoneand every probe falls through to the PowerShell branch. Functionally this is handled correctly (fallback works), but since this platform-detection code is explicitly called out as pending real Windows hardware validation, swapping the probe order (PowerShell CIM first, wmic as legacy fallback) avoids a doomed-first-attempt on the majority of current installs.♻️ Suggested reorder
commands = [ - ["wmic", "path", "win32_VideoController", "get", "name"], ["powershell", "-NoProfile", "-Command", "Get-CimInstance Win32_VideoController | Select-Object -ExpandProperty Name"], + ["wmic", "path", "win32_VideoController", "get", "name"], ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/mc-setup/scripts/check_deps.py` around lines 90 - 110, Reverse the command order in gpu_names_windows so the PowerShell CIM probe runs before the legacy wmic probe. Keep the existing availability checks, subprocess handling, output parsing, and wmic fallback behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/mc-ograf/scripts/verify_ograf.py`:
- Around line 77-100: Update manual_verify_steps to quote the pkg path when
constructing cd_cmd for Windows, Darwin, and other systems, while preserving the
existing platform-specific cd syntax and command text.
---
Nitpick comments:
In `@skills/mc-audio/scripts/farm_audio.py`:
- Around line 76-88: Remove the duplicated venv_python() definition from
skills/mc-audio/scripts/farm_audio.py#L76-L88 and
skills/mc-audio/scripts/ensure_workspace.py, move it to a shared platform_utils
module, and import it in both callers while preserving OS-specific paths. In
skills/mc-audio/scripts/tests/test-ensure_workspace.py#L30-L65 and
skills/mc-audio/scripts/tests/test-farm_audio.py#L29-L92, replace each local
make_venv_shim() definition with an import from one shared test helper so both
suites use identical fixture logic.
In `@skills/mc-setup/scripts/check_deps.py`:
- Around line 90-110: Reverse the command order in gpu_names_windows so the
PowerShell CIM probe runs before the legacy wmic probe. Keep the existing
availability checks, subprocess handling, output parsing, and wmic fallback
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4ad4d31-2a54-4567-b38d-2d8dbef093c8
⛔ Files ignored due to path filters (1)
skills/module-help.csvis excluded by!**/*.csv
📒 Files selected for processing (45)
CHANGELOG.mdREADME.mdTODO.mddocs/index.htmldocs/manny-under-the-hood.htmldocs/user-guide.mdskills/mc-assets/SKILL.mdskills/mc-assets/scripts/farm_asset.pyskills/mc-assets/scripts/tests/test-farm_asset.pyskills/mc-audio/SKILL.mdskills/mc-audio/scripts/engines/music_musicgen.pyskills/mc-audio/scripts/engines/sfx_audioldm2.pyskills/mc-audio/scripts/ensure_workspace.pyskills/mc-audio/scripts/farm_audio.pyskills/mc-audio/scripts/tests/test-ensure_workspace.pyskills/mc-audio/scripts/tests/test-farm_audio.pyskills/mc-cut/SKILL.mdskills/mc-cut/scripts/composite_core.pyskills/mc-cut/scripts/edl_to_fcpxml.pyskills/mc-cut/scripts/preflight.pyskills/mc-cut/scripts/render_final.pyskills/mc-cut/scripts/tests/test-edl_to_fcpxml.pyskills/mc-cut/scripts/tests/test-preflight.pyskills/mc-cut/scripts/tests/test-render_final.pyskills/mc-cut/scripts/tests/test-transcribe.pyskills/mc-cut/scripts/transcribe.pyskills/mc-ograf/SKILL.mdskills/mc-ograf/references/resolve-workflow.mdskills/mc-ograf/scripts/tests/test-verify_ograf.pyskills/mc-ograf/scripts/verify_ograf.pyskills/mc-package/SKILL.mdskills/mc-package/scripts/captions.pyskills/mc-package/scripts/tests/test-captions.pyskills/mc-pipeline/PIPELINE.mdskills/mc-setup/SKILL.mdskills/mc-setup/assets/formats/livestream-pack.mdskills/mc-setup/customize.tomlskills/mc-setup/references/stack-linux.mdskills/mc-setup/references/stack-macos.mdskills/mc-setup/references/stack-windows.mdskills/mc-setup/scripts/check_deps.pyskills/mc-setup/scripts/tests/test-check_deps.pyskills/mc-stream-pack/SKILL.mdskills/mc-stream-pack/scripts/render_verify.pyskills/mc-stream-pack/scripts/tests/test-render_verify.py
…before legacy wmic Addresses CodeRabbit review on #7: paths with spaces no longer break the copy-paste manual steps in verify_ograf.py, and Windows GPU detection tries Get-CimInstance first since wmic is disabled by default on Windows 11 23H2+ and slated for removal.
Contributor
Author
|
Responses to the CodeRabbit review, addressed in 2820d15:
Both affected test suites re-run green (10 + 20 tests). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Makes the Manticore pipeline fully cross-platform (code-complete for Windows, Linux, and Intel Mac, pending real-hardware validation) and lands the Mac-ready enhancements from the 2026-07-21 platform research.
Cross-platform port
autoprovider picks parakeet-mlx on Apple Silicon (byte-identical to the 1.0 lane) and onnx-asr everywhere else. PEP 508 markers select dependencies per platform; long audio is chunked (20 s windows, 2 s overlap) with a seam-repair merge; words.json stays byte-compatible across lanes.Added (works everywhere, validated on macOS)
[render]loudness-target, disable with loudnorm = false or --no-loudnorm. Previews are never normalized.Verification
README platform matrix, user guide, PIPELINE.md, TODO, CHANGELOG (Unreleased), and the docs/ pages are updated to match.
Summary by CodeRabbit
New Features
autoprovider selection, deterministic chunking/merging, and consistent timestamps/confidence handling.Documentation
Bug Fixes