Dubbing: per-language subtitle scripts and export_srt - #40
Merged
Merged
Conversation
`POST /v1/dubbing` takes a target-language script per language as the repeatable multipart field `subtitles[<language>]` (an https URL or an uploaded .srt/.vtt), and `export_srt` returns a re-timed .srt per language once the dub is delivered. `lipsync` has existed on the endpoint all along and never reached this SDK. `build_dubbing_parts` now returns a ready-made `close_after` as its third element instead of an `opened` bool, the shape `build_ducking_parts` already uses. One request can open the video plus one script per target language, and a bool cannot say how many handles that is — nor which of them this builder opened. Every handle is closed on the way out if a later open fails, and the caller closes the rest through the `close_after` seam `_post_json` already exposes, so the routine 422 from a rejected script set does not leak them. Client-side checks are limited to the two guaranteed 422s: a script path whose suffix is not .srt/.vtt, and `export_srt` with no scripts. The language set is not checked against `languages` — the server owns that rule, its message names the offending code, and a local copy would break a caller who relies on the server default without passing `languages` at all. `DubbingResult` gains `subtitles` (with save_subtitle / save_all_subtitles mirroring save / save_all), `subtitle_preflight` and `subtitle_export`. The two report maps are coerced but their values are left as they arrived: the pipeline stores numbers as strings, so `cue_count` may be "5" and `alignment_loss` "0.63", and guessing at a type here would only mislead. A blocked export does not fail the task, so `subtitles` can lack a language whose video was still delivered — save_all_subtitles iterates `subtitles`, not `outputs`. `lipsync` defaults ON server-side and every dubbing task ran that way before the field existed, so it is sent only when the caller passes it: absent has to keep meaning true. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
`--subtitle <language>=<path-or-url>`, repeated once per target language, carries the script to speak in it; `--export-srt` asks for a re-timed .srt back. The `=` is split once only so a path or URL containing one survives. Neither the codes nor the language set are checked here — the server owns both rules and its message names what it refused; the SDK still refuses a suffix that is not .srt/.vtt, which is a guaranteed 422. Each .srt is written beside its video (clip.es.mp4 -> clip.es.srt) and every requested language gets a status line, including a language whose export was blocked: that still delivers the video, just without a script. The alignment loss is formatted through float() because the pipeline stores numbers as strings — "0.63" and 0.63 both have to print — and is omitted when it is neither. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
New parameters on an existing endpoint, so a minor bump on both packages — and `__version__` is bumped alongside `pyproject.toml` in each, since it ships as the `x-sonilo-client-version` header and CI now fails on a one-sided bump (#39). The CLI's `sonilo>=0.15.0,<0.16` pin has to widen to `>=0.16.0,<0.17` in this same commit: it calls `dubbing.generate(subtitles=..., export_srt=...)`, which only exists from 0.16.0, and left alone the pin would make the editable install of both packages unresolvable. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
The dubbing section of each README gains the new parameters: the target- language scripts and what the server checks about them, the re-timed .srt `export_srt` returns, the three new maps on `DubbingResult` (including that a blocked export still delivers the video), and `lipsync`, which has been on the endpoint all along and was documented nowhere. Also f-strings the two `subtitles[<language>]` field names, matching the rest of the module. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
The JavaScript CLI's dubbing command has had --ducking since the ducking round and has just gained --no-lipsync; the two CLIs mirror each other, so this side was the one out of step. --ducking goes through the same `_ducking()` resolution the sound commands use, so --no-ducking keeps parsing as the explicit-default no-op it is elsewhere and passing both still exits 1. Only --no-lipsync exists, with no positive counterpart: lip sync is default-ON server-side, so the sole useful direction is turning it off, and an absent flag sends nothing rather than restating a default that could move. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
`submit()` parsed the dubbing ack with the shared `parse_sfx_task`, which builds an `SfxTask` of `task_id` and `status` — so the whole per-language `subtitle_preflight` map was read off the wire and dropped. That map is the free, pre-charge check: a language whose status is `review_required` had lines changed in the script the caller submitted, and with `submit()` there was no way to see it before the billed dub finished. Adds `DubbingTask`, an additive subclass of `SfxTask` carrying the map, and `parse_dubbing_task` to fill it — coerced through the same `_report_map_from` the finished task's copy uses. `SfxTask` itself is untouched: it acks every other async endpoint, and the preflight is dubbing's alone. Mirrors the JavaScript SDK's `DubbingTask extends SfxTask`. Also drops the unreachable `or "subtitles.srt"` fallback in `_subtitle_filename` — an empty name has an empty suffix, so the extension check raises before it — and fixes the README example, which read `report["status"]` two lines under its own advice to read these defensively. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
…tests `sonilo dubbing --export-srt --output clip.srt` wrote the dubbed video to clip.es.srt and then wrote the subtitle over it, printing a "Wrote" line for each: both files come from the one template and the subtitle goes second, so the mp4 was silently truncated away. It is refused at parse time now, before anything runs, with a message naming the cause and the fix — matched case-insensitively, and the same refusal the JavaScript CLI makes. The three tests that asserted only an exit code plus a flag name were passing against `main` for the wrong reason: argparse's own "unrecognized arguments" error quotes the flag, so they would not have caught a regression that deleted the flags outright. Each now matches the message this CLI actually prints. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
The branch was cut from a stale main and re-implemented `lipsync`, which #37 had already shipped and #38's release published as sonilo 0.16.0 / sonilo-cli 0.15.0. Every lipsync conflict is resolved in favour of upstream: their parameter (and its position in `build_dubbing_parts`), their comments, their class docstring, their `--no-lipsync` flag and its help text, their test and both README paragraphs. Mine are deleted rather than merged — there is one implementation here, and it is theirs. My `--ducking`/`--no-ducking` on `sonilo dubbing` stays: upstream added no positive flag there, so it is still genuinely missing, and the CLI test that covered both now covers ducking only. The subtitle work is kept whole, rebased onto their signature: `lipsync` keeps its upstream position and `subtitles`/`export_srt` follow it. Versions are re-picked, since 0.16.0 and 0.15.0 are taken: sonilo 0.17.0 and sonilo-cli 0.16.0, both files each. The CLI's pin moves from upstream's `sonilo>=0.16.0,<0.17` to `>=0.17.0,<0.18` — it calls `subtitles=` and `export_srt=`, which only exist from 0.17.0. Claude-Session: https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk
Lightsage docs evalsWaiting for the staging docs URL before running evals. Lightsage will start the selected PR evals automatically when GitHub reports a successful docs deployment for this PR. This usually happens within 15 minutes. Commit: |
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
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.
POST /v1/dubbinggained two parameters, both live in production. This brings them to the client and the CLI.subtitles— one script per target languageA dubbing job can now carry the exact lines you want spoken, instead of the pipeline translating for you. One script per language being dubbed into, as
subtitles[<language>]on the wire.A value is a string or a
Path: one starting withhttps://is sent as a URL, anything else is a local path, opened and uploaded. A suffix that is not.srtor.vttis refused before the request goes out.The set of languages must match
languagesexactly, but that rule is not copied client-side: the server owns it and its message names the offending code. Language codes are likewise not validated here, for the same reason the existing code gives.export_srtOff by default, and requires
subtitles. On, each language's final audio is force-aligned against its script and a re-timed SRT comes back with the lines preserved verbatim.save_subtitleandsave_all_subtitlesfetch them, mirroringsaveandsave_all, with async twins.An export can be blocked for one language without failing the job. That language keeps its video and simply has no SRT.
save_all_subtitlesskips it;save_subtitleraises naming the language and its status.The CLI writes the SRT beside the video:
--output clip.mp4givesclip.ja.mp4andclip.ja.srt. An--outputending in.srtis refused with--export-srt, since the subtitle would land on the video's own path.File handles
build_dubbing_partscan now open several files instead of one, so its third return value went from anopenedbool to theMultiClosethatbuild_ducking_partsalready uses. Every path was checked for leaks: a raise partway through the opens, a raise from the HTTP call, and the routine 422 all close every handle, in both the sync and async clients.Numbers arrive as strings
The finished task carries
cue_countandalignment_lossas strings rather than numbers. The reports are passed through as received rather than coerced, so the values are whatever the server sent, and the CLI formats the loss defensively.Preflight
submit()now returns aDubbingTask, which adds the per-language preflight report the 202 has always carried. Areview_requiredstatus means the pipeline changed lines in the script you sent.SfxTaskis untouched — it is shared with every other endpoint.--duckingon the CLIsonilo dubbingnever exposed it, though the JavaScript CLI has since the ducking round. Added here so the two CLIs mirror each other.Tests
Core 316 passing, CLI 167 passing, on both Python 3.9 and 3.12. The new tests were checked against the pre-change code and fail there.
sonilo0.17.0,sonilo-cli0.16.0, and the CLI's pin widened to match. Publishsonilofirst — the CLI hard-requiressonilo>=0.17.0, so the reverse order leaves a window wherepip install sonilo-clicannot resolve.🤖 Generated with Claude Code
https://claude.ai/code/session_01WWHfnbRaXRdAH2sjm8mApk