From: Carl Hetherington Date: Sun, 6 Dec 2020 18:55:07 +0000 (+0100) Subject: Add some hints for violations of SMPTE Bv2.1 with subtitles and closed X-Git-Tag: 11.0-build-runs~63 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=504e234f92de578a8d1214d6a73c790a896f0a90 Add some hints for violations of SMPTE Bv2.1 with subtitles and closed captions. --- diff --git a/src/lib/hints.cc b/src/lib/hints.cc index 14022bb59..23d5a15fb 100644 --- a/src/lib/hints.cc +++ b/src/lib/hints.cc @@ -60,6 +60,11 @@ Hints::Hints (weak_ptr film) , _long_ccap (false) , _overlap_ccap (false) , _too_many_ccap_lines (false) + , _early_subtitle (false) + , _short_subtitle (false) + , _subtitles_too_close (false) + , _too_many_subtitle_lines (false) + , _long_subtitle (false) , _stop (false) { @@ -368,33 +373,90 @@ Hints::hint (string h) void Hints::text (PlayerText text, TextType type, DCPTimePeriod period) { - if (type != TEXT_CLOSED_CAPTION) { - return; + switch (type) { + case TEXT_CLOSED_CAPTION: + closed_caption (text, period); + break; + case TEXT_OPEN_SUBTITLE: + open_subtitle (text, period); + break; + default: + break; } +} + +void +Hints::closed_caption (PlayerText text, DCPTimePeriod period) +{ int lines = text.string.size(); BOOST_FOREACH (StringText i, text.string) { - if (utf8_strlen(i.text()) > CLOSED_CAPTION_LENGTH) { + if (utf8_strlen(i.text()) > MAX_CLOSED_CAPTION_LENGTH) { ++lines; if (!_long_ccap) { _long_ccap = true; - hint (String::compose(_("Some of your closed captions have lines longer than %1 characters, so they will probably be word-wrapped."), CLOSED_CAPTION_LENGTH)); + hint ( + String::compose( + "At least one of your closed caption lines has more than %1 characters. " + "It is advisable to make each line %1 characters at most in length.", + MAX_CLOSED_CAPTION_LENGTH, + MAX_CLOSED_CAPTION_LENGTH) + ); } } } - if (!_too_many_ccap_lines && lines > CLOSED_CAPTION_LINES) { - hint (String::compose(_("Some of your closed captions span more than %1 lines, so they will be truncated."), CLOSED_CAPTION_LINES)); + if (!_too_many_ccap_lines && lines > MAX_CLOSED_CAPTION_LINES) { + hint (String::compose(_("Some of your closed captions span more than %1 lines, so they will be truncated."), MAX_CLOSED_CAPTION_LINES)); _too_many_ccap_lines = true; } /* XXX: maybe overlapping closed captions (i.e. different languages) are OK with Interop? */ - if (film()->interop() && !_overlap_ccap && _last && _last->overlap(period)) { + if (film()->interop() && !_overlap_ccap && _last_ccap && _last_ccap->overlap(period)) { _overlap_ccap = true; hint (_("You have overlapping closed captions, which are not allowed in Interop DCPs. Change your DCP standard to SMPTE.")); } - _last = period; + _last_ccap = period; +} + + +void +Hints::open_subtitle (PlayerText text, DCPTimePeriod period) +{ + if (period.from < DCPTime::from_seconds(4) && !_early_subtitle) { + _early_subtitle = true; + hint (_("It is advisable to put your first subtitle at least 4 seconds after the start of the DCP to make sure it is seen.")); + } + + int const vfr = film()->video_frame_rate (); + + if (period.duration().frames_round(vfr) < 15 && !_short_subtitle) { + _short_subtitle = true; + hint (_("At least one of your subtitles lasts less than 15 frames. It is advisable to make each subtitle at least 15 frames long.")); + } + + if (_last_subtitle && DCPTime(period.from - _last_subtitle->to).frames_round(vfr) < 2 && !_subtitles_too_close) { + _subtitles_too_close = true; + hint (_("At least one of your subtitles starts less than 2 frames after the previous one. It is advisable to make the gap between subtitles at least 2 frames.")); + } + + if (text.string.size() > 3 && !_too_many_subtitle_lines) { + _too_many_subtitle_lines = true; + hint (_("At least one of your subtitles has more than 3 lines. It is advisable to use no more than 3 lines.")); + } + + size_t longest_line = 0; + BOOST_FOREACH (StringText const& i, text.string) { + longest_line = max (longest_line, i.text().length()); + } + + if (longest_line > 52 && !_long_subtitle) { + _long_subtitle = true; + hint (_("At least one of your subtitle lines has more than 52 characters. It is advisable to make each line 52 characters at most in length.")); + } + + _last_subtitle = period; } @@ -415,3 +477,10 @@ Hints::check_ffec_and_ffmc_in_smpte_feature () hint (_("SMPTE DCPs with the type FTR (feature) should have markers for the first frame of end credits (FFEC) and the first frame of moving credits (FFMC). You should add these markers using the 'Markers' button in the DCP tab.")); } } + + +void +Hints::join () +{ + _thread.join (); +} diff --git a/src/lib/hints.h b/src/lib/hints.h index b5a26998d..e10037763 100644 --- a/src/lib/hints.h +++ b/src/lib/hints.h @@ -44,10 +44,17 @@ public: boost::signals2::signal Pulse; boost::signals2::signal Finished; + /* For tests only */ + void join (); + private: + friend struct hint_subtitle_too_early; + void thread (); void hint (std::string h); void text (PlayerText text, TextType type, dcpomatic::DCPTimePeriod period); + void closed_caption (PlayerText text, dcpomatic::DCPTimePeriod period); + void open_subtitle (PlayerText text, dcpomatic::DCPTimePeriod period); boost::shared_ptr film () const; void check_big_font_files (); @@ -69,7 +76,14 @@ private: bool _long_ccap; bool _overlap_ccap; bool _too_many_ccap_lines; - boost::optional _last; + boost::optional _last_ccap; + + bool _early_subtitle; + bool _short_subtitle; + bool _subtitles_too_close; + bool _too_many_subtitle_lines; + bool _long_subtitle; + boost::optional _last_subtitle; boost::atomic _stop; }; diff --git a/src/lib/util.h b/src/lib/util.h index 91ce4b443..548dd0475 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -62,9 +62,9 @@ namespace dcp { /** Largest KDM size (in bytes) that will be accepted */ #define MAX_KDM_SIZE (256 * 1024) /** Number of lines that closed caption viewers will display */ -#define CLOSED_CAPTION_LINES 3 +#define MAX_CLOSED_CAPTION_LINES 3 /** Maximum line length of closed caption viewers, according to SMPTE Bv2.1 */ -#define CLOSED_CAPTION_LENGTH 32 +#define MAX_CLOSED_CAPTION_LENGTH 32 extern std::string program_name; extern bool is_batch_converter; diff --git a/src/wx/closed_captions_dialog.cc b/src/wx/closed_captions_dialog.cc index 5522e5c4c..e37bd5910 100644 --- a/src/wx/closed_captions_dialog.cc +++ b/src/wx/closed_captions_dialog.cc @@ -58,7 +58,7 @@ ClosedCaptionsDialog::ClosedCaptionsDialog (wxWindow* parent, FilmViewer* viewer , _current_in_lines (false) , _timer (this) { - _lines.resize (CLOSED_CAPTION_LINES); + _lines.resize (MAX_CLOSED_CAPTION_LINES); wxBoxSizer* sizer = new wxBoxSizer (wxVERTICAL); @@ -104,16 +104,16 @@ ClosedCaptionsDialog::paint () dc.SetTextForeground (*wxWHITE); /* Choose a font which fits vertically */ - int const line_height = max (8, dc.GetSize().GetHeight() / CLOSED_CAPTION_LINES); + int const line_height = max (8, dc.GetSize().GetHeight() / MAX_CLOSED_CAPTION_LINES); wxFont font (*wxNORMAL_FONT); font.SetPixelSize (wxSize (0, line_height * 0.8)); dc.SetFont (font); - for (int i = 0; i < CLOSED_CAPTION_LINES; ++i) { - wxString const good = _lines[i].Left (CLOSED_CAPTION_LENGTH); + for (int i = 0; i < MAX_CLOSED_CAPTION_LINES; ++i) { + wxString const good = _lines[i].Left (MAX_CLOSED_CAPTION_LENGTH); dc.DrawText (good, 8, line_height * i); - if (_lines[i].Length() > CLOSED_CAPTION_LENGTH) { - wxString const bad = _lines[i].Right (_lines[i].Length() - CLOSED_CAPTION_LENGTH); + if (_lines[i].Length() > MAX_CLOSED_CAPTION_LENGTH) { + wxString const bad = _lines[i].Right (_lines[i].Length() - MAX_CLOSED_CAPTION_LENGTH); wxSize size = dc.GetTextExtent (good); dc.SetTextForeground (*wxRED); dc.DrawText (bad, 8 + size.GetWidth(), line_height * i); @@ -158,7 +158,7 @@ ClosedCaptionsDialog::update () if (_current && _current->period.to < time) { /* Current one has finished; clear out */ - for (int j = 0; j < CLOSED_CAPTION_LINES; ++j) { + for (int j = 0; j < MAX_CLOSED_CAPTION_LINES; ++j) { _lines[j] = ""; } Refresh (); @@ -192,7 +192,7 @@ ClosedCaptionsDialog::update () list to_show = _current->text.string; - for (int j = 0; j < CLOSED_CAPTION_LINES; ++j) { + for (int j = 0; j < MAX_CLOSED_CAPTION_LINES; ++j) { _lines[j] = ""; } @@ -200,7 +200,7 @@ ClosedCaptionsDialog::update () list::const_iterator j = to_show.begin(); int k = 0; - while (j != to_show.end() && k < CLOSED_CAPTION_LINES) { + while (j != to_show.end() && k < MAX_CLOSED_CAPTION_LINES) { _lines[k] = j->text(); ++j; ++k; @@ -211,7 +211,7 @@ ClosedCaptionsDialog::update () } if (!_current && _tracks.empty()) { - for (int i = 0; i < CLOSED_CAPTION_LINES; ++i) { + for (int i = 0; i < MAX_CLOSED_CAPTION_LINES; ++i) { _lines[i] = wxString(); } } diff --git a/test/data b/test/data index 1458a693f..66e5e6e6a 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 1458a693f163e82ffb8d2b81b9abdad1fb8acb1e +Subproject commit 66e5e6e6a7bd9fe817a00011c1c1c32c6f4e01c7 diff --git a/test/hints_test.cc b/test/hints_test.cc new file mode 100644 index 000000000..0e47125d0 --- /dev/null +++ b/test/hints_test.cc @@ -0,0 +1,147 @@ +/* + Copyright (C) 2020 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + + +#include "lib/content.h" +#include "lib/content_factory.h" +#include "lib/film.h" +#include "lib/hints.h" +#include "lib/text_content.h" +#include "lib/util.h" +#include "test.h" +#include +#include + + +using std::string; +using std::vector; +using boost::shared_ptr; + + +vector current_hints; + + +static +void +collect_hint (string hint) +{ + current_hints.push_back (hint); +} + + +static +vector +get_hints (shared_ptr film) +{ + current_hints.clear (); + Hints hints (film); + hints.Hint.connect (collect_hint); + hints.start (); + hints.join (); + while (signal_manager->ui_idle()) {} + return current_hints; +} + + +static +void +check (TextType type, string name, string expected_hint) +{ + shared_ptr film = new_test_film2 (name); + shared_ptr content = content_factory("test/data/" + name + ".srt").front(); + content->text.front()->set_type (type); + film->examine_and_add_content (content); + BOOST_REQUIRE (!wait_for_jobs()); + vector hints = get_hints (film); + + BOOST_REQUIRE_EQUAL (hints.size(), 1); + BOOST_CHECK_EQUAL (hints[0], expected_hint); +} + + +BOOST_AUTO_TEST_CASE (hint_closed_caption_too_long) +{ + check ( + TEXT_CLOSED_CAPTION, + "hint_closed_caption_too_long", + String::compose("At least one of your closed caption lines has more than %1 characters. It is advisable to make each line %1 characters at most in length.", MAX_CLOSED_CAPTION_LENGTH, MAX_CLOSED_CAPTION_LENGTH) + ); +} + + +BOOST_AUTO_TEST_CASE (hint_many_closed_caption_lines) +{ + check ( + TEXT_CLOSED_CAPTION, + "hint_many_closed_caption_lines", + String::compose("Some of your closed captions span more than %1 lines, so they will be truncated.", MAX_CLOSED_CAPTION_LINES) + ); +} + + +BOOST_AUTO_TEST_CASE (hint_subtitle_too_early) +{ + check ( + TEXT_OPEN_SUBTITLE, + "hint_subtitle_too_early", + "It is advisable to put your first subtitle at least 4 seconds after the start of the DCP to make sure it is seen." + ); +} + + +BOOST_AUTO_TEST_CASE (hint_short_subtitles) +{ + check ( + TEXT_OPEN_SUBTITLE, + "hint_short_subtitles", + "At least one of your subtitles lasts less than 15 frames. It is advisable to make each subtitle at least 15 frames long." + ); +} + + +BOOST_AUTO_TEST_CASE (hint_subtitles_too_close) +{ + check ( + TEXT_OPEN_SUBTITLE, + "hint_subtitles_too_close", + "At least one of your subtitles starts less than 2 frames after the previous one. It is advisable to make the gap between subtitles at least 2 frames." + ); +} + + +BOOST_AUTO_TEST_CASE (hint_many_subtitle_lines) +{ + check ( + TEXT_OPEN_SUBTITLE, + "hint_many_subtitle_lines", + "At least one of your subtitles has more than 3 lines. It is advisable to use no more than 3 lines." + ); +} + + +BOOST_AUTO_TEST_CASE (hint_subtitle_too_long) +{ + check ( + TEXT_OPEN_SUBTITLE, + "hint_subtitle_too_long", + "At least one of your subtitle lines has more than 52 characters. It is advisable to make each line 52 characters at most in length." + ); +} + diff --git a/test/wscript b/test/wscript index cf0e8e6e3..f47eaea73 100644 --- a/test/wscript +++ b/test/wscript @@ -83,6 +83,7 @@ def build(bld): film_metadata_test.cc frame_interval_checker_test.cc frame_rate_test.cc + hints_test.cc image_content_fade_test.cc image_filename_sorter_test.cc image_test.cc