Fix V4+ Style Format for Aegisub Compatibility#133
Open
tassa-yoniso-manasi-karoto wants to merge 2 commits into
Open
Fix V4+ Style Format for Aegisub Compatibility#133tassa-yoniso-manasi-karoto wants to merge 2 commits into
tassa-yoniso-manasi-karoto wants to merge 2 commits into
Conversation
* reorder updateFormat() fields to canonical V4+ order * Aegisub and other parsers ignore the Format line and parse by position * previous alphabetical order caused "bad int field" parse errors
* always include all 22 canonical fields regardless of nil values * provide Aegisub defaults for nil fields instead of empty strings * fixes "bad int field" errors from missing fields in format line
Owner
|
First off thanks for the PR ❤️
Don't forget to fix tests if they're not passing anymore 👍 |
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.
Problem 1: Field Order
When writing ASS (V4+) files,
updateFormat()builds the Styles Format line by adding fields in alphabetical order (Alignment, AlphaLevel, Angle, BackColour, Bold, ...). However, Aegisub ignores the Format line entirely and parses style values by fixed position. Other parsers (libass, VSFilter) likely follow Aegisub's conventions as the de facto standard.This causes a mismatch: the Format line says position 2 is "Alignment", but parsers expect position 2 to be "Fontname". When Aegisub tries to parse a font name as an integer (for the Bold field later in the sequence), it throws:
Problem 2: Missing Fields
Even after fixing field order,
updateFormat()only adds fields that have non-nil values. If a field likeStrikeOutis nil (e.g., after a deep copy operation), it's omitted from the Format line entirely. This shifts all subsequent field positions, causing the same parse errors.Additionally,
string()outputs empty strings for nil fields, which is invalid (e.g., empty string where Aegisub expects "0" or "1" for a boolean).Commit Scope Clarification
Commit 1 (Field Order): Affects all users. Any ASS file written by go-astisub currently fails to parse in Aegisub due to alphabetical field order.
Commit 2 (Nil Defaults): Affects edge cases where style fields are nil, such as deep copying with reflection-based libraries or programmatically creating styles without initializing all fields. Under typical read-modify-write workflows, fields remain populated.
I encountered this via
jinzhu/copierfor deep copying, where pointer fields like*boolbecame nil. Regardless of how common this scenario is, go-astisub should not output corrupt ASS files when fields are nil. It should use sensible defaults rather than omitting fields or outputting empty strings.Design Decision: V4+ (ASS) Only
This PR intentionally targets V4+ (ASS) only and removes support for writing legacy V4 (SSA) format:
Rationale is that according to Gemini 3 Pro:
.ssaextension often contain V4+ content anywayIf a rare legacy V4 file is encountered, reading still works (unchanged), but output is always upgraded to V4+. There is no benefit to writing old V4 files today.
Spec vs Reality
The written SSA V4+ (ASS) specification states that the Format line "allows new fields to be added... even if the field order is changed." However, the reference implementation (Aegisub) does not implement this flexibility: it parses style values by fixed position, ignoring the Format line entirely. This PR aligns with real-world behavior. See gist for Aegisub source code evidence.
References