feat(subtitles): keep text subtitle styling and placement to the host - #239
Merged
Merged
Conversation
Re #233, for everything the source layer can actually deliver. The styling was never lost in transit. libavcodec converts every text subtitle format into an ASS event line before the engine sees it, so the markup was on AVSubtitleRect.ass the whole time: SRT through ff_htmlmarkup_to_ass, WebVTT through its own tag table, teletext because the decoder already runs txt_format=ass. What discarded it was ours, cleanASSBody stripping every {...} block and the colour parser handling \c and documenting that it ignored the rest. So this is one parser, not one per format. coloredRuns becomes styledRuns, and SRT, WebVTT, teletext and ASS light up together. The teletext positioning that was arriving and being dropped comes with it. SubtitleTextRun gains isBold, isItalic, isUnderlined, isStruckThrough, fontName and fontSize; SubtitleCue gains placement, an ASS numpad alignment plus an optional anchor normalized like SubtitleImage.position is. New initializer parameters are defaulted, so no call site changes. \pos normalizes against a declared PlayResX/Y when the ASS header has one, otherwise libavcodec's 384x288 default, which is what the lines it synthesises use. Unconditional, no gate. An unstyled cue still arrives as .text carrying the same string, so a host handling only that case sees nothing new, and .richText was already reachable through teletext. The PiP compositor was flattening rich text to a joined string in one white font, so the styling would have stopped at the PiP window. It now builds a per-run attributed line (colour, face, ASS-relative size, real CoreText bold and italic traits, underline, and a drawn rule for strikeout since CoreText has no attribute for it) and honours placement. Not covered, and not coverable here: WebVTT cue settings never arrive, libavcodec does not convert them and webvttdec.c says so in its file header. VTT inline styling works; VTT positioning needs an upstream patch. SRT positioning needs AV_PKT_DATA_SUBTITLE_POSITION side data that most files lack. The #107 teletext blank-line fold moved out of the run parser into teletextBody, where it belongs: only libzvbi's row joining produces those, and a blank line in an ASS or SRT cue can be deliberate. 1210 tests green, strict-concurrency=complete clean, tvOS and iOS Simulator BUILD SUCCEEDED. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01At1agC2qU2Y2MX2BKLdT4N
SidecarASSMarkupTests is an XCTest suite, so its failure did not appear
in the Swift Testing summary I checked locally and CI caught it instead.
The assertion was correct for the old behaviour: the default sidecar path
flattened {\i1}Hello{\i0} world to plain text.
That is the behaviour #233 deliberately changes, and this test is a
useful end-to-end proof of it, since the fixture also declares
PlayResX/Y and so exercises the header play-resolution parsing. It now
asserts the new contract: still header-less, still no raw event lines,
but a styled cue arrives as .richText with the italic resolved and an
unstyled one stays .text. The flattened SubtitleCue.text is asserted
unchanged, which is what a host reading plain text actually sees.
XCTest 392 tests 0 failures, Swift Testing 1210 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01At1agC2qU2Y2MX2BKLdT4N
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.
Summary
Re #233. Text subtitle styling (bold, italic, underline, strikeout, colour, font face and size) and the placement a cue asks for now reach the host instead of being stripped at the parser.
What changed
AVSubtitleRect.ass: SRT throughff_htmlmarkup_to_ass, WebVTT through its own tag table, teletext because the decoder already rantxt_format=ass.cleanASSBodystripped every{...}block and the colour parser handled\cand ignored the rest.coloredRunsbecomesstyledRuns, a full override parser, and SRT, WebVTT, teletext and ASS light up together.SubtitleTextRungainsisBold,isItalic,isUnderlined,isStruckThrough,fontName,fontSize.SubtitleCuegainsplacement(SubtitleTextPlacement: ASS numpad alignment plus an optional anchor normalized to [0, 1] likeSubtitleImage.position). New initializer parameters are defaulted, so no call site changes.\posnormalization uses a declaredPlayResX/Ywhen the ASS header has one, otherwise libavcodec's 384x288 default, which is what its synthesised lines use..textwith the same string, so a host handling only that case sees no change;.richTextwas already reachable through teletext.\be/\bordare not\b,\iclipis not\i,\shadis not\s,\fscx/\fspare not\fs.teletextBody, where it belongs: only libzvbi's row joining produces those, and a blank line in an ASS or SRT cue can be deliberate.Not covered
WebVTT cue settings (
line,position,align,size,vertical) never arrive: libavcodec does not convert them,webvttdec.csays so in its file header. VTT inline styling works, VTT positioning needs an upstream patch. SRT positioning needsAV_PKT_DATA_SUBTITLE_POSITIONside data most files lack.Test plan
Issue233ASSOverrideStyleTests(21): attributes, toggles, multi-tag blocks, ASS weights, font name/size and their resets, lookalike tags,\r, colour regressions, cue-level\an/\pos, play-res normalization and header parsing, SRT- and VTT-shaped lines, body selection, escapes, and the teletext fold boundary.Issue233CompositorStylingTests(12): placement maths for all nine alignments with and without an explicit position, font trait and size resolution, render smoke tests.TeletextColourRunsTestsmoved onto the new API with every AU live tv #107 assertion preserved.strict-concurrency=completeclean, tvOS and iOS Simulator BUILD SUCCEEDED.Checklist
CHANGELOG.mdupdatedREADME.mdupdated