Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#10194 closed Patch - Feature (fixed)

Add formatting support for SRT subtitles

Reported by: Jim Stichnoth <stichnot@…> Owned by: danielk
Priority: minor Milestone: 0.25
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

This patch adds support for formatting controls in SRT subtitles. This includes:

<i> italics </i>
<b> boldface </b>
<u> underline </u>
<font color="#xxyyzz"> color </font>

It also refactors the EIA-608 formatting code to use the same mechanism. It should be fairly applicable for EIA-708 formatting once support is added to the cc708 decoder/reader.

Attachments (12)

srt_formatting_v1.patch (24.2 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
srt_formatting_v2.patch (25.6 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Remove redundant code for setting font size and attributes.
srt_formatting_v3.patch (26.1 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Two bug fixes. 1: ensure shape sizes are computed wrt correct font pixel size. 2: when wrapping long lines, the iterator becomes invalid after a new line is added.
srt_formatting_v4.patch (26.1 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Remove a duplicate increment.
srt_formatting_v5.patch (26.1 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Small tweak to vertical spacing so SRT lines are separated by a small (20%) gap like cc608 captions.
srt_formatting_v6.patch (26.2 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Updated after subtitlescreen.cpp checkins.
srt_cc708_v1.patch (51.1 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Add cc708 captions to the refactoring. Leverage for mythccextractor.
srt_cc708_v2.patch (50.9 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Remove accidental debug logging.
srt_cc708_v3.patch (51.3 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Minor cleanup. Restored some cc708 logging from the original code.
srt_cc708_v4.patch (52.4 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Enhance logging to include full style info, e.g. fg/bg color/alpha, font, edge style, etc.
srt_cc708_v5.patch (51.2 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Updated to latest master.
srt_cc708_v6.patch (51.8 KB) - added by Jim Stichnoth <stichnot@…> 12 years ago.
Change new static fields/methods back to instance. gCC708Fonts is left unchanged for now.

Download all attachments as: .zip

Change History (22)

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v1.patch added

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v2.patch added

Remove redundant code for setting font size and attributes.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v3.patch added

Two bug fixes. 1: ensure shape sizes are computed wrt correct font pixel size. 2: when wrapping long lines, the iterator becomes invalid after a new line is added.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v4.patch added

Remove a duplicate increment.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v5.patch added

Small tweak to vertical spacing so SRT lines are separated by a small (20%) gap like cc608 captions.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_formatting_v6.patch added

Updated after subtitlescreen.cpp checkins.

comment:1 Changed 12 years ago by danielk

Milestone: unknown0.25
Owner: set to danielk
Status: newassigned

Jim can document here how I can produce formatted SRT files to test with?

i.e. what sample stream can I use and what program should I run on it to get the SRT files?

Thanks

comment:2 Changed 12 years ago by Jim Stichnoth <stichnot@…>

You can use ccextractor (http://ccextractor.sourceforge.net/) on a suitable U.S. ATSC recording to get an SRT file with italics, colors, and underlining. Italics are reasonably common, especially for off-screen dialog. Color changes are very rare, usually only for sponsorship messages. I don't recall ever seeing underlining in a clean recording.

I can upload a small sample mpeg with plenty of italics if needed.

For testing, I manually edit the resulting SRT file to add boldface, underlining, and colors. I tested combinations of attributes, and also very long lines to maintain the existing line-wrapping functionality.

The most "official" documentation of SRT formatting that I can find is http://forum.doom9.org/showthread.php?p=470941#post470941 . This patch doesn't support SRT display coordinates. It also doesn't try to correctly support multi-level nesting of attributes, e.g. </i> always turns off italics, instead of restoring the previous state, e.g. the following would be displayed without italics:

<i> <i></i> This should be in italics. </i>

See the comments at the beginning of FormattedTextSubtitle::InitFromSRT() for a description of this implementation.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v1.patch added

Add cc708 captions to the refactoring. Leverage for mythccextractor.

comment:3 Changed 12 years ago by Jim Stichnoth <stichnot@…>

Refactored CEA-708 caption display to use the same underlying mechanism.

Modified mythccextractor to use the infrastructure for CEA-608 and CEA-708 streams, allowing SRT formatting attributes to be output.

This patch also fixes the CEA-708 issues in #10283 and #10291.

Part of the refactoring uses the font at index 0 of the cc708 font array to replace gTextSubFont which was used for text and cc608 captions.

This has been tested across cc608, cc708, and text subtitles with different values of CCBackground and OSDCC708TextZoom (the latter applies only to cc708 and text subs).

Note: the srt_cc708_*.patch is designed to be applied on top of the srt_formatting_*.patch.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v2.patch added

Remove accidental debug logging.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v3.patch added

Minor cleanup. Restored some cc708 logging from the original code.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v4.patch added

Enhance logging to include full style info, e.g. fg/bg color/alpha, font, edge style, etc.

comment:4 Changed 12 years ago by Github

Refs #10194. Adds support for formatting controls in SRT subtitles.

Branch: master Changeset: b63d887cd847566cec3d9afc9280f215801ac8dd

comment:5 Changed 12 years ago by danielk

Jim, the srt_cc708_v4.patch doesn't apply anymore.. Can you also break out the fix for large text and anything else not related to the formatting.

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v5.patch added

Updated to latest master.

comment:6 Changed 12 years ago by Jim Stichnoth <stichnot@…>

After b63d887cd847566cec3d9afc9280f215801ac8dd, srt_formatting_v*.patch is no longer needed. srt_cc708_v5.patch is updated accordingly.

The patch now contains just the refactoring to support CEA-708 captions, plus the mythccextractor part (mythccextractorplayer.{cpp,h} modifications and the ToSRT() method). The font size issue from #10283 happens to be fixed as a result of the refactoring.

comment:7 Changed 12 years ago by danielk

Milestone: 0.250.26

Jim, I think making Get708Font(CC708CharacterAttribute attr) a static and making m_708fontSizes static is really a regression. It may work when we only have one video playing, but I don't want to tie the subtitle screen to presenting captions for just one video at a time. I'm also suspicious any time a function returning a pointer or reference is made public; in this case it could probably remain private but FormattedTextSubtitle? could be made a friend of SubtitleScreen?.

PS I realize gCC708Fonts has the same problem, but I'd rather fix that than further entrench it. If we need to cache that across runs, there has to be a better way..

Changed 12 years ago by Jim Stichnoth <stichnot@…>

Attachment: srt_cc708_v6.patch added

Change new static fields/methods back to instance. gCC708Fonts is left unchanged for now.

comment:8 Changed 12 years ago by Github

Milestone: 0.260.25
Resolution: fixed
Status: assignedclosed

Fixes #10194. Fixes #10283. Refactor of 708 caption handling.

Note there are a couple minor issues. The default OSDSubFont is not as readable as the old 708 default font and the font can not be reset without restarting mythfrontend. These issues aren't really caused by this changeset though, so I'm applying this as is and leaving those issues to be fixed in a subsequent patch.

Branch: master Changeset: 8038cb11da4e4f803e88f4476d9e49348bb3ca19

comment:9 Changed 12 years ago by Github

Allow OSDSubFont changes to take effect without restarting mythfrontend.

Static fields and methods in the SubtitleScreen? class are changed to instance fields/methods so that font initialization is done once per instance rather than once per application. The performance reasons for one-time initialization went away when the OSD was converted to MythUI.

Further refactoring/cleanup is due post-0.25.

Refs #10194

Branch: master Changeset: ccee693ce875b9f19486f0db90eebdcf9872164c

comment:10 Changed 12 years ago by Github

Subtitles: improve the layout.

With the existing PAD_WIDTH value, there was still some clipping going on, for example an italic "W" at the end of the line with the Droid Sans Mono font. Simply increasing PAD_WIDTH would have made the extra spacing between adjacent but differently-formatted text chunks much more noticeable.

This is improved by increasing PAD_WIDTH but no longer adding padding between chunks. A consequence is that the background rectangle now has the possibility of clipping the text to its left, so there is a final step that moves all text objects to the front (rather than drawing background and text in separate passes).

Refs #10194.

Branch: master Changeset: 5d5aef36c3d1037c295ed7b915ee12b3a8773e10

Note: See TracTickets for help on using tickets.