OCTO-11541 - #399
Closed
OlteanuRares wants to merge 2 commits into
Closed
Conversation
…ting
between formats with different visual defaults.
VTT/SRT/SCC visually default to center; DFXP/SAMI default to left/start.
Without explicit alignment, cross-format conversion causes visual
regression. The fix detects source format via null layouts and the new
is_positional_anchor flag, and emits explicit center in DFXP/SAMI output.
- DFXP writer: emit tts:textAlign=center for VTT/SRT/SCC sources;
suppress tts:origin from SCC positional layouts
- SAMI writer: emit text-align:center for centerdefault sources;
use semantic <i>/<b>/<u> tags; add trailing clearing sync
- Geometry: add is_positional_anchor to Layout (SCC row/col vs alignment)
- Tests: update assertions for center alignment and semantic tags
🟡 PR Compliance ReviewRisk Level: MEDIUM
REVIEW REQUIRED - Address issues before merging Full report available in workflow artifacts |
…ttings SCC row/col coordinates leaked as align:left position:N% line:N% size:N% in VTT output — a visual regression where centered SCC captions appeared left-aligned in VTT players. The is_positional_anchor check now returns early from _convert_positioning(), letting VTT default to center.
🟡 PR Compliance ReviewRisk Level: MEDIUM
REVIEW REQUIRED - Address issues before merging Full report available in workflow artifacts |
Contributor
Author
|
solved with #400 |
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.
Summary
is_positional_anchorand mapped to center instead of leakingtts:origininto DFXP,margin-left/margin-topinto SAMI, oralign:left position:N% line:N%into VTT<i>/<b>/<u>tags and adds a trailing sync to clear the final captionProblem
DFXP's spec default is
textAlign="start"(left). SAMI's default istext-align: left. When a VTT/SRT/SCC file (visual default: center) was converted to DFXP or SAMI without explicit alignment, players rendered the text left-aligned — a silent visual regression with no data loss to detect.SCC captions carry origin+LEFT alignment as a coordinate-system anchor (row 14, col 0), not as "align this text left." Previous code emitted these as
tts:originin DFXP,margin-left/margin-topin SAMI, andalign:left position:N% line:N% size:N%in VTT — producing incorrect positioning in all three targets.Approach
Layout.is_positional_anchorflag distinguishes SCC coordinate positioning from visual alignment intent (set in_get_layout_from_tuple)RegionCreator._has_null_layouts()detects VTT/SRT/SCC sources (no layout → center default region) vs DFXP/SAMI sources (has layout → preserve original alignment)_is_scc_positional()gates origin suppression in DFXP and SAMI writers_has_null_layouts()returns False)SinglePositioningDFXPWriter,LegacyDFXPWriter) are unaffected — they assign layout before reaching the new logicTest plan
tts:textAlign="center"present, notts:textAlign="left", notts:origintext-align:centerpresent, no margin leaksalign:left, noposition:, noline:, nosize:align:start/align:left(preserves source intent)start/left(no false promotion)align:left/align:rightpreserved through conversionAlignment matrix — all 25 conversion paths (implicit default input)