Repository navigation
SCC: small, independent fixes found with the CEA-608 conformance suite (1/6) - #558
Open
dheitmueller wants to merge 8 commits into
Open
dheitmueller wants to merge 8 commits into
dheitmueller wants to merge 8 commits into
Conversation
CTA-608-E describes the vertical bar and the four corners (Tables 9 and 10, p.24-25, notes 8 and 9) as characters that extend to the limits of their cell, to draw boxes with the em dash, and the character 0x7F as a solid block (Table 50, p.98). The standard gives no code points. The reader used the heavy box drawing characters U+2503 and U+250F, U+2513, U+2517 and U+251B, which are in far fewer fonts than the light ones, and U+2588 for the block. Use U+2502, U+250C, U+2510, U+2514 and U+2518, the light characters that do the same, and U+25A0, the black square that the standard prints. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 6 (C01c, 00:28), test 12 (C03e, 01:02) and test 13 (C03f, 01:08). Update the tests of the characters.
A row has 32 columns (CTA-608-E C.13, p.75), so the writer reflows a line that is longer. The search for the space to break at compared a character with the integer that ord() returns, which is never equal, so every line was cut at column 32, in the middle of a word, and the character at the cut was dropped as if it were the space: "The quick brown fox jumps over the lazy dog today" was written as "The quick brown fox jumps over t" and "e lazy dog today". Break at the last space that leaves at most 32 characters in the row, and drop only that space. A word without a space in its first 32 characters is cut at column 32 and keeps every character. Found by converting a TTML document with a line of more than 32 characters to SCC and reading it back. The same on upstream master. Add tests that fail without this change.
A row that a CR creates in a roll-up caption has no attributes until a PAC assigns them: its text is white, not underlined and not italic (CTA-608-E C.14, p.75). The reader kept the color, the underline and the italics of the row above, so the row that follows a yellow underlined row was yellow underlined too. Reset the attributes that the next text receives when a CR is read. Reproduced with the CEA-608 conformance suite v1.2 (CEA608-conformance-v1.2-720p59-h264.ts), the field 1 pairs written as an SCC file: test 34 (A08, 02:42), row 2 must be white and not underlined. Add a test that fails without this change.
A control code is sent twice in successive frames, and the second one is ignored (CTA-608-E D.2, p.78). A code that is repeated after other data, a null pair for example, is a new command (CEA-CEB-10-A 25.2.4, p.41). The reader ignored every control word that was equal to the previous one, so BS, null, BS erased one character instead of two. Keep the time of the previous word and ignore a control word only if it is in the frame that follows that word. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 63 (D09, 07:14), the caption must end as ABCD after the second BS, it ended as ABCDE. Add a test that fails without this change.
When the style is changed to roll-up, a pop-on or paint-on caption that is in either memory is erased (CTA-608-E C.10, p.74). The reader erased what was displayed, but left the caption that was loaded in the non-displayed memory, which was shown by the next EOC. Erase the buffered caption too when RU2, RU3 or RU4 switches to roll-up. A paragraph that has no content gives its identifier back, so that the identifiers of the output stay in sequence. The displayed pop-on caption now ends at the frame of the RU command, not at the one of the control code after it (the EDM or the CR), and a roll-up caption that starts with CR begins at the CR. Two existing tests expected the times one frame later, and are updated. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 62 (D08, 06:58), the hidden caption B must never be shown. Add a test that fails without this change.
CTA-608-E gives the extended characters 0x12 0x2A and 0x13 0x2C as the em dash (Table 6, p.21) and the caret (Table 8, p.23). The standard gives the names and the glyphs of the characters, not code points. The reader mapped the em dash to U+2501, BOX DRAWINGS HEAVY HORIZONTAL, and the caret to U+028C, LATIN SMALL LETTER TURNED V, which is only drawn like a caret. Use the characters that have the names of the standard, U+2014 EM DASH and U+005E CIRCUMFLEX ACCENT, which are also in all common fonts. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 9 (C03b, 00:44) and test 11 (C03d, 00:56). Update the tests of the extended characters.
A Foreground Attribute code (black, black underline) changes the color of the characters only; the background that a Background Attribute code set remains until the end of the row or until another background code (CTA-608-E 6.2, p.16-17). The reader started a new text with the default background, so black text after a white background was black on black. The text that follows the code also took the color of the last PAC or mid-row code instead of the one of the code. Carry the background over to the new text, and make the color and the decoration of the code the current ones. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 37 (A09c, 02:59), "Black text on white". Add a test that fails without this change.
Of three consecutive CRs the second is redundant (CTA-608-E D.2, p.78), so two rows roll up and one blank row remains between the captions. The reader treated a CR as nothing to roll up as soon as the current row was empty, even when rows were displayed above it, so the second roll was lost. Roll up unless nothing at all is displayed. Reproduced with the CEA-608 conformance suite v1.2, the field 1 pairs written as an SCC file: test 54 (R11, 05:43), A must be two rows above B. Add a test that fails without this change.
Contributor
|
@dheitmueller Thanks... and you will probably hate what I am about to request: can you open a separate issue for each of the commits? |
Contributor
Author
|
@palemieux So I can certainly open separate issues for each if that makes it easier for you. It is worth noting though that in some cases patches are dependent on prior patches in the series, so I'm not confident you'll be able to merge fixes for the issues in any arbitrary order. |
Contributor
|
I think we might be able to keep the PR as one, but it would be good to have each issues separate so we can keep track of the discussion on each individually. |
Contributor
Contributor
Author
|
Opened Issues 560 through 566 for the items in this patch series. |
Contributor
|
@dheitmueller I think this PR has been replaced by individual PRs. I would appreciate your review on the latter. |
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.
This is the first of a few pull requests that fix problems in the SCC reader and writer that I found by running the CEA-608 Conformance Suite (v1.2) through ttconv. The suite is a set of test streams with a reference decoder; for ttconv I wrote its field 1 captions as an SCC file, converted it to IMSC and to SCC, and compared what each caption looks like with what the reference decoder displays, test by test.
Each commit below names the test that shows the problem (for example "test 62 (D08, 06:58)") and the section of CTA-608-E (or CEA-CEB-10-A) that defines the behavior.