Fix incorrect WebVTT size mapping for horizontal TTML text - #143
Open
quocngo-dek wants to merge 5 commits into
Open
Fix incorrect WebVTT size mapping for horizontal TTML text#143quocngo-dek wants to merge 5 commits into
quocngo-dek wants to merge 5 commits into
Conversation
The bug was in propagateTTMLAttributes() where WebVTTSize was incorrectly set to the height dimension (dimensions[1]) by default, when it should use the width dimension (dimensions[0]) for horizontal text. For TTML with default WritingMode "lrtb" (left-to-right, top-to-bottom), the WebVTT size: property should represent the horizontal extent (width). Only for vertical writing modes (starting with "tb") should it use height. Changes: - subtitles.go: Swapped the dimension assignments to use width by default and height only for vertical writing modes - ttml_test.go: Updated test expectations to reflect correct behavior (WebVTTSize now correctly set to "100%" width instead of height values) This ensures TTML extent="80% 15%" correctly maps to WebVTT size:80% for horizontal text, not size:15%. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Author
|
Hi @asticode, please help me take a look on this PR. Thanks for your attention |
Changed from incorrect single-line format: Region: id=fred width=40% lines=3 regionanchor=0%,100% To correct multi-line format: REGION id:fred width:40% lines:3 regionanchor:0%,100% According to W3C WebVTT specification, REGION blocks must use: - Multi-line block format (not single-line) - Colon separators (setting:value, not setting=value) - Line breaks between each setting The previous format was non-standard and may not be parsed correctly by compliant WebVTT parsers. Reference: https://www.w3.org/TR/webvtt1/#webvtt-region-settings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changes: 1. RegionAnchor: Changed from "0%,0%" (top-left) to "0%,100%" (bottom-left) 2. ViewportAnchor: Calculate bottom edge position (origin Y + extent height) instead of using origin directly Rationale: According to W3C WebVTT spec, the default regionanchor is "0%,100%" (bottom-left corner), which is appropriate for bottom-aligned subtitle regions commonly used in TTML with displayAlign="after". For bottom-aligned TTML text, the viewport anchor should be positioned at the bottom edge of the region, not the top edge. Since TTML origin specifies the top-left corner, we calculate the bottom edge as: bottom edge Y = origin Y + extent height Example: TTML: origin="0% 80%" extent="100% 20%" Region spans from 80% to 100% (bottom 20% of viewport) WebVTT: regionanchor:0%,100% viewportanchor:0%,100% This ensures TTML bottom-aligned subtitles remain anchored at the bottom in WebVTT instead of floating upward from the top of the region. References: - https://www.w3.org/TR/webvtt1/#webvtt-region-anchor-setting - https://w3c.github.io/ttml-webvtt-mapping/ Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updated test expectations to reflect the three bug fixes: 1. TTML test (ttml_test.go): - WebVTTRegionAnchor: "0%,0%" → "0%,100%" (bottom-left anchor) - WebVTTViewportAnchor: Updated to bottom edge calculations - WebVTTScroll: "up" → "" (removed automatic scroll setting) 2. WebVTT output test (testdata/example-out.vtt): - REGION blocks now use multi-line format with colons - Changed from: Region: id=X lines=Y ... - Changed to: REGION\nid:X\nlines:Y\n... All tests now pass with the corrected W3C-compliant implementations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
nvkhoi112358
left a comment
nvkhoi112358
left a comment
There was a problem hiding this comment.
Please include a test for the inline Region:.
Added ability to read the new W3C-compliant REGION format while maintaining backward compatibility with the old Region: format. Now supports both: 1. NEW format (W3C spec): REGION id:fred width:40% lines:3 scroll:up 2. OLD format (backward compatibility): Region: id=fred width=40% lines=3 scroll=up This ensures round-trip compatibility: - Read old WebVTT files with "Region:" → Works ✓ - Read new WebVTT files with "REGION" → Works ✓ - Write always outputs new format → W3C compliant ✓ - Read back what we wrote → Works ✓ The parser detects "REGION" and switches to multi-line mode, parsing each setting with colon separators. Empty lines finalize the region block. Reference: https://www.w3.org/TR/webvtt1/#webvtt-region-settings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Author
|
Author
|
hi @asticode , I am still waiting for your comments. Please take a look on this PR |
Contributor
|
hi @asticode , could you please take a look in this PR? |
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
3 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.
Fixed WebVTT size mapping for horizontal TTML text
When converting TTML to WebVTT, the code incorrectly used the height dimension for the WebVTT
size:property by default, when it should use width for horizontal text.Example:
TTML:
extent="80% 15%"→ WebVTT:size:15%❌Should be:
extent="80% 15%"→ WebVTT:size:80%✅Fixed WebVTT REGION block format to match W3C specification
REGION blocks were output in an incorrect single-line format that doesn't match the W3C WebVTT specification.
Before (incorrect):
Region: id=fred width=40% lines=3 regionanchor=0%,100% scroll=upAfter (correct):
Bottom-aligned TTML subtitles were incorrectly positioned in WebVTT output, causing them to float upward instead of staying anchored at the bottom.
Correct Example:
Region:format, breaking round-trip conversion.Solution: Added parser support for the new
REGIONformat while maintaining backward compatibility.Now Supports Both Formats:
Region: id=fred ...(old)REGIONmulti-line (new)References: