Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 2 years ago

Closed 2 years ago

Last modified 2 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@…> 2 years ago.
srt_formatting_v2.patch (25.6 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
Remove redundant code for setting font size and attributes.
srt_formatting_v3.patch (26.1 KB) - added by Jim Stichnoth <stichnot@…> 2 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@…> 2 years ago.
Remove a duplicate increment.
srt_formatting_v5.patch (26.1 KB) - added by Jim Stichnoth <stichnot@…> 2 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@…> 2 years ago.
Updated after subtitlescreen.cpp checkins.
srt_cc708_v1.patch (51.1 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
Add cc708 captions to the refactoring. Leverage for mythccextractor.
srt_cc708_v2.patch (50.9 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
Remove accidental debug logging.
srt_cc708_v3.patch (51.3 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
Minor cleanup. Restored some cc708 logging from the original code.
srt_cc708_v4.patch (52.4 KB) - added by Jim Stichnoth <stichnot@…> 2 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@…> 2 years ago.
Updated to latest master.
srt_cc708_v6.patch (51.8 KB) - added by Jim Stichnoth <stichnot@…> 2 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 2 years ago by Jim Stichnoth <stichnot@…>

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

Remove redundant code for setting font size and attributes.

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

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 2 years ago by Jim Stichnoth <stichnot@…>

Remove a duplicate increment.

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

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

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

Updated after subtitlescreen.cpp checkins.

comment:1 Changed 2 years ago by danielk

  • Milestone changed from unknown to 0.25
  • Owner set to danielk
  • Status changed from new to assigned

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 2 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 2 years ago by Jim Stichnoth <stichnot@…>

Add cc708 captions to the refactoring. Leverage for mythccextractor.

comment:3 Changed 2 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 2 years ago by Jim Stichnoth <stichnot@…>

Remove accidental debug logging.

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

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

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

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

comment:4 Changed 2 years ago by Github

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

Branch: master
Changeset: b63d887cd847566cec3d9afc9280f215801ac8dd

comment:5 Changed 2 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 2 years ago by Jim Stichnoth <stichnot@…>

Updated to latest master.

comment:6 Changed 2 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 2 years ago by danielk

  • Milestone changed from 0.25 to 0.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 2 years ago by Jim Stichnoth <stichnot@…>

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

comment:8 Changed 2 years ago by Github

  • Milestone changed from 0.26 to 0.25
  • Resolution set to fixed
  • Status changed from assigned to closed

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 2 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 2 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

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.