Fix CEA-608 preamble access codes that indent.

[faginbagin <mythtv@…>]

PACs that indent were off by one column. Without the fix, the indent will be a multiple of 4 minus 1. For example: an indent of 4 will be an indent of 3, an indent of 8 will be an indent of 7, etc.

The problem was caused by inconsistent treatment of unicode chars in the range 0x7000 to 0x7030. These codes are used to mark attribute changes in a QString, and sometimes, but not always spaces. The fix was to change them so they ONLY mark attribute changes, and spaces are added as needed to increment the column position.

To verify the problem and test the fix:

• add the following video clip to your video storage group: ​http://ncamftp.wgbh.org/DTV/CEA%20test%20material/Iteration_1/CEAv1.2zero.trp Be sure to change the filename extension from .trp to .mpg,
• Scan for changes.
• Start to play the clip, then pause.
• Press M/menu. Select Subtitles -> Select VBI CC -> CC 1: English
• Play and fast forward to LTCR 01:02:10:00 (a timecode in video)
• Use play/pause to carefully observe the "Indent" captions.
• When the caption "Indent0 UL" is displayed, take note of the location of the second "n". It is in column 4.
• Resume play, then pause when the next caption, "Indent4", is displayed. The "I" should be in the same position as the second "n" of the previous caption. Without the fix, it will instead be in column 3, the same location as the previous caption's "e".

The attached patch should apply cleanly to either fixes/0.27 or master branches. FWIW, I tested it on a modified fixes/0.27 that contains the latest versions from master of these files: mythtv/libs/libmythtv/cc608decoder.cpp mythtv/libs/libmythtv/cc708reader.cpp mythtv/libs/libmythtv/cc708window.cpp mythtv/libs/libmythtv/cc708window.h mythtv/libs/libmythtv/mythccextractorplayer.cpp mythtv/libs/libmythtv/subtitlescreen.cpp mythtv/libs/libmythtv/subtitlescreen.h

[Jim Stichnoth]

Just for context, the 0x70nn characters are interpreted as spaces primarily so that backspace and tab processing can be handled simply (though I suspect tabs aren't currently correct).

Back when this was first implemented in #9772, I was unhappy with the way preamble codes created an initial space character, which would show up against a black background when the background was enabled. This was an ugliness shared by my STB's caption renderer. This is why there is inconsistent treatment of these unicode characters. However, I think later caption work turns leading spaces into an indent without background for all caption types.

In any case, simply changing the assignment of limit to "int limit = newcol[mode];" seems to fix the indent problem, and I don't see any ill effects on other captions in the WGBH sample or other recordings. Do you see any issues with that?

[Jim Stichnoth <jstichnoth@…>]

In 7d60440b2d790208a25ce707480f5b01657410f1/mythtv:

Subtitles: Fix cc608 indents.

The original code was trying to be clever and indent one character
less than intended presumably to account the extra space character
that control codes also imply, but it's not clear that was ever an
appropriate thing to do.

Thanks to faginbagin@ for identifying the problem and the fix.
Refs #12051.

[faginbagin <mythtv@…>]

So I guess what you're saying is my other changes could mess up roll-up captions that use PACs and then backspace. Yeah, I can see that's possible.

FWIW, I think there might have been value once upon a time in implementing cc608 captions using a 2D array with chars and attributes as opposed to using a string with spaces and newlines, but it would be a lot of work to refactor the code, and what's there works well enough 99% of the time.

I'll test your one line change and report whether I see any issues.

[faginbagin <mythtv@…>]

Well, I can't find a sample that justifies all the other changes in my patch that change the 0x70xx codes into attribute only markers. Your one line changes seems to be all that's needed.

[Jim Stichnoth <jstichnoth@…>]

In 57699052e6eb16293a5954ad4312331448d9ef10/mythtv:

Subtitles: Fix cc608 indents.

The original code was trying to be clever and indent one character
less than intended presumably to account the extra space character
that control codes also imply, but it's not clear that was ever an
appropriate thing to do.

Thanks to faginbagin@ for identifying the problem and the fix.
Refs #12051.
(cherry picked from commit 7d60440b2d790208a25ce707480f5b01657410f1)

[Jim Stichnoth]

Replying to faginbagin <mythtv@…>:

Well, I can't find a sample that justifies all the other changes in my patch that change the 0x70xx codes into attribute only markers. Your one line changes seems to be all that's needed.

Heh, that was actually your one-line change. :)

Thanks for checking on this.

[faginbagin <mythtv@…>]

I'm afraid I have found a sample that does justify all the other changes in my patch. It's a case where the captioner adds preamble access codes and tab offsets that really aren't needed before mid row codes that turn on and off italics. I've even been able to cull the sample down to 1.5 Mbytes, so I should be able to upload it to trac.

The caption is displayed by myth as:

THAT
     <i>THIS</i>
          REACTION IS
<i>BECAUSE</i>
        YOU DIDN'T TELL ME?

It should be displayed as

THAT <i>THIS</i> REACTION IS
<i>BECAUSE</i> YOU DIDN'T TELL ME?

[Jim Stichnoth <jstichnoth@…>]

In 366c428708682420bbf53a301d145ca441aedbdd/mythtv:

Subtitles: Fix cc608 preamble indents. Fixes #12051.

Thanks to Helen (faginbagin) for the analysis and patch.

As Helen points out in the ticket, it would be all-around better if we
took an approach like the cc708 captions and rendered everything onto
an internal grid of characters and then converted that to strings to
draw on the screen, but that's a lot of work for a mostly deprecated
caption format.

[Jim Stichnoth <jstichnoth@…>]

In 45d2d5173aa1cd27ef15180b4afe3e4779d26019/mythtv:

Subtitles: Fix cc608 preamble indents. Fixes #12051.

Cherry-picked from 366c428708682420bbf53a301d145ca441aedbdd .

Thanks to Helen (faginbagin) for the analysis and patch.

As Helen points out in the ticket, it would be all-around better if we
took an approach like the cc708 captions and rendered everything onto
an internal grid of characters and then converted that to strings to
draw on the screen, but that's a lot of work for a mostly deprecated
caption format.