Repository navigation
fix(dub): keep newer subtitles when a render or transcription finishes late - #2614
Merged
Merged
Conversation
Setup progress keyed downloads by the raw announced name, so a package spelled pydantic-core on one line and pydantic_core on the next lost its progress and completion. Names are now PEP 503 normalized for matching while the announced spelling is still shown.
A cancel that arrived while a task committed its result made the worker drop the done event and mark the task cancelled, so clients could redo work that already existed. A done update is now always delivered.
Source subtitle replacements (SRT import, downloaded captions, cleanup, transcription) now advance a per-job revision. A dub render publishes its track, segments and manifest only if the revision and cancellation state are unchanged, so imported corrections are never overwritten and the previous track survives a late cancel. Fresh segment speech is staged and enters the partial-regeneration cache only together with its fingerprint. Transcription commits its transcript and voice references atomically and yields to subtitles imported while it ran. Published tracks drop QC marks measured on the previous track, and empty dub requests are rejected.
# Conflicts: # CHANGELOG.md
Contributor
|
[Medium risk] Fixes subtitle and audio sync timing in the dubbing pipeline. The PR appears safe to merge based on the reviewed changes and resolved previous findings. SummaryThe PR protects newer subtitles and saved voice references from late dubbing work, publishes rendered audio with its cache metadata, and updates task, setup, and renderer behavior. Since the previous review, it also adds model-licence records and credits, adjusts Pro descriptions, and packages licence notices. The three previous Greptile threads are resolved; no new merge-changing finding was established. Reviews (5) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (44)
🚧 Files skipped from review as they are similar to previous changes (43)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe changes add revision-checked transcription commits and staged dubbing publication. They preserve existing output when generation is cancelled or source subtitles change, clear prior quality-check marks, handle completion events during cancellation, validate non-empty dub requests, and normalize package names in setup progress tracking. ChangesDubbing integrity
Setup download progress
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Possibly related PRs
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No identified issue blocks merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Revision checks and staged publication strengthen protection against stale work replacing newer subtitles or audio. No introduced or worsened security issue was established in the inspected paths. Interrupted-work cleanup and coordination across multiple running processes remain incompletely established. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 38.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 92 functions across 16 files. (43 skipped: 43 unsupported.) Full details: Cross-Platform Default ParityExplanation FAIL — default dub rendering can leave different segment caches on Windows. In Resolution Make cache publication independent of open-file replacement semantics. For example, write segments to immutable versioned paths and atomically switch the cache manifest with the track, or defer publication until replacement can complete; do not publish a track with platform-dependent cache state. Add a regression test for a held-open cache file and verify the same user-visible result on macOS, Windows, and Linux.
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 |
… published edits - A render cancelled or superseded during assembly had already moved its speech and fingerprints into the shared segment cache, so previews read through the previous track's manifest played rejected audio. Staged speech now reaches the cache in the publication commit. - A published dub replaced the job's segments without advancing the revision transcription checks. Every segment write now goes through the revision helpers, with a separate write counter so renders of other languages are not rejected. - Each transcription writes its voice references to its own folder, so cues imported mid-pass keep their clips; commits replace the previous transcript's clone maps. Existing jobs keep their references in place. - The import-during-transcription test now uses asr_model_installed: without ASR weights the stream ended at the model-missing preflight, which is why it failed only in CI. A guard test requires that stub in every transcription test.
…ribe the new behaviour
…on cleans up The import selected the references it keeps outside the job lock, so a transcription finishing meanwhile could commit, delete the previous pass's reference folder, and leave the import saving paths to deleted clips. Selection and save are now one locked step, and the folder cleanup runs under the same lock and keeps any folder the job's saved references still point into.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
phvhao
added a commit
to phvhao/VoiceStudio
that referenced
this pull request
Oct 8, 2026
…onunciation Brings in the 206 upstream commits up to 06c6e07: the 0.5.7 release, cross-site and Host checks, local-only ffmpeg inputs, offload to RAM after generation (debpalash#2618), dub subtitle integrity (debpalash#2614), the Pro licence cleanup and the 2026-10b issue batch. Conflicts keep both sides: - dub_generate: upstream's staged publication (seg_order) with our Smart Fit video_speed_cap and per-line video_fit in the fit fingerprint. - dub_background: our shared retime helper builds each chunk with upstream's retime_chunk_filter (debpalash#2616); zero-length cues stay skipped. - ipc: streamed, abortable saves report upstream's encoded download failure. - replace-file: upstream's shared rename-retry with our streamed data. - Locales: upstream's licence wording and new keys, our Vietnamese wording elsewhere; keys upstream removed stay removed. - Docs: service count, both features' notes. Follow-ups the merge needs: - The true-peak meter's ffmpeg argv goes through local_inputs_only. - The one-shot HTML export download refuses cross-site requests, like upstream's other side-effecting GET routes.
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
2 participants
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.
Replaces #2585 with a maintainer-only reimplementation of its review findings.
What changes
DubRequest.segmentsreturns 422 instead of replacing the track with silence.-/_spellings.Each fix has a regression test that fails on the old code. The changelog,
docs/electron-dubbing.mdand all locales are updated.Not included
maindiscards it today as well).Dubbing now preserves the existing track when source subtitles change during generation, stages speech with its fingerprints before publication, and commits transcription results only if source subtitles remain unchanged. The changes also handle cancellation after work is saved, reject empty
DubRequest.segments, and match setup progress across package-name variants. A known risk remains: importing subtitles during reference extraction can overwrite reference audio in the job folder.