From 937e4352072486832372fc8ebdb83583be9b8a2a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 13 Jun 2016 23:11:20 +0100 Subject: More consistent error message in a subtitle exception. --- src/smpte_subtitle_asset.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/smpte_subtitle_asset.cc b/src/smpte_subtitle_asset.cc index c7272066..685b4fc7 100644 --- a/src/smpte_subtitle_asset.cc +++ b/src/smpte_subtitle_asset.cc @@ -99,7 +99,7 @@ SMPTESubtitleAsset::SMPTESubtitleAsset (boost::filesystem::path file) } catch (cxml::Error& e) { boost::throw_exception ( DCPReadError ( - String::compose ("could not read subtitles from %1; MXF failed with %2, XML failed with %3", file, static_cast (r), e.what ()) + String::compose ("MXF failed with %1, XML failed with %2", file, static_cast (r), e.what ()) ) ); } -- cgit v1.2.3 From 8837fe70049bd5ed7ab583c06d6c9a620a01b351 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 14 Jun 2016 00:20:06 +0100 Subject: Tighten up time parsing, and also allow the previously unsupported Interop HH:MM:SS.sss format for times. --- src/dcp_time.cc | 82 +++++++++++++++++++++++++++++++++++++------ src/dcp_time.h | 3 +- src/font_node.cc | 4 +-- src/font_node.h | 4 +-- src/interop_subtitle_asset.cc | 4 +-- src/subtitle_node.cc | 9 ++--- src/subtitle_node.h | 6 ++-- src/text_node.cc | 4 +-- src/text_node.h | 4 +-- test/dcp_time_test.cc | 35 ++++++++++++++++++ 10 files changed, 127 insertions(+), 28 deletions(-) (limited to 'src') diff --git a/src/dcp_time.cc b/src/dcp_time.cc index 7425df2c..401abb3c 100644 --- a/src/dcp_time.cc +++ b/src/dcp_time.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -38,7 +38,9 @@ #include "raw_convert.h" #include "dcp_time.h" #include "exceptions.h" +#include "compose.hpp" #include +#include #include #include #include @@ -91,20 +93,80 @@ Time::set (double seconds, int tcr_) } } -/** @param time String of the form HH:MM:SS:EE[E] */ -Time::Time (string time, int tcr_) - : tcr (tcr_) +/** @param time String of the form + * HH:MM:SS:EE for SMPTE + * HH:MM:SS:E[E[E]] or HH:MM:SS.s[s[s]] for Interop + * where HH are hours, MM minutes, SS seconds, EE editable units and + * sss millseconds. + * + * @param tcr_ Timecode rate if this is a SMPTE time, otherwise empty for an Interop time. + */ +Time::Time (string time, optional tcr_) { vector b; split (b, time, is_any_of (":")); - if (b.size() != 4) { - boost::throw_exception (DCPReadError ("unrecognised time specification")); + + if (b.size() < 3 || b[0].empty() || b[1].empty() || b[0].length() > 2 || b[1].length() > 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1", time))); } - h = raw_convert (b[0]); - m = raw_convert (b[1]); - s = raw_convert (b[2]); - e = raw_convert (b[3]); + if (!tcr_) { + /* Interop */ + if (b.size() == 3) { + /* HH:MM:SS.s[s[s]] */ + vector bs; + split (bs, b[2], is_any_of (".")); + if (bs.size() != 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1", time))); + } + + h = raw_convert (b[0]); + m = raw_convert (b[1]); + if (bs[0].empty() || bs[0].length() > 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, bs[0]))); + } + s = raw_convert (bs[0]); + if (bs[1].empty() || bs[1].length() > 3) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, bs[1]))); + } + e = raw_convert (bs[1]); + tcr = 1000; + } else if (b.size() == 4) { + /* HH:MM:SS:EE[E] */ + h = raw_convert (b[0]); + m = raw_convert (b[1]); + if (b[2].empty() || b[2].length() > 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, b[2]))); + } + s = raw_convert (b[2]); + if (b[3].empty() || b[3].length() > 3) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, b[3]))); + } + e = raw_convert (b[3]); + tcr = 250; + } else { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1", time))); + } + + } else { + /* SMPTE: HH:MM:SS:EE */ + split (b, time, is_any_of (":")); + if (b.size() != 4) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; does not have 4 parts", time))); + } + + h = raw_convert (b[0]); + m = raw_convert (b[1]); + if (b[2].empty() || b[2].length() > 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, b[2]))); + } + s = raw_convert (b[2]); + if (b[3].empty() || b[3].length() > 2) { + boost::throw_exception (DCPReadError (String::compose ("unrecognised time specification %1; %2 has bad length", time, b[3]))); + } + e = raw_convert (b[3]); + tcr = tcr_.get(); + } } bool diff --git a/src/dcp_time.h b/src/dcp_time.h index e8724614..2a9f13d8 100644 --- a/src/dcp_time.h +++ b/src/dcp_time.h @@ -39,6 +39,7 @@ #define LIBDCP_TIME_H #include "types.h" +#include #include #include #include @@ -78,7 +79,7 @@ public: Time (double seconds, int tcr); - Time (std::string time, int tcr); + Time (std::string time, boost::optional tcr); int h; ///< hours int m; ///< minutes diff --git a/src/font_node.cc b/src/font_node.cc index 77d58aaf..6dd6e58f 100644 --- a/src/font_node.cc +++ b/src/font_node.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -45,7 +45,7 @@ using boost::shared_ptr; using boost::optional; using namespace dcp; -FontNode::FontNode (cxml::ConstNodePtr node, int tcr, string font_id_attribute) +FontNode::FontNode (cxml::ConstNodePtr node, optional tcr, string font_id_attribute) { text = node->content (); diff --git a/src/font_node.h b/src/font_node.h index 72c1553e..8f7cc2aa 100644 --- a/src/font_node.h +++ b/src/font_node.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -54,7 +54,7 @@ public: : size (0) {} - FontNode (cxml::ConstNodePtr node, int tcr, std::string font_id_attribute); + FontNode (cxml::ConstNodePtr node, boost::optional tcr, std::string font_id_attribute); explicit FontNode (std::list > const & font_nodes); std::string text; diff --git a/src/interop_subtitle_asset.cc b/src/interop_subtitle_asset.cc index 33d76e89..b32ac790 100644 --- a/src/interop_subtitle_asset.cc +++ b/src/interop_subtitle_asset.cc @@ -68,12 +68,12 @@ InteropSubtitleAsset::InteropSubtitleAsset (boost::filesystem::path file) list > font_nodes; BOOST_FOREACH (cxml::NodePtr const & i, xml->node_children ("Font")) { - font_nodes.push_back (shared_ptr (new FontNode (i, 250, "Id"))); + font_nodes.push_back (shared_ptr (new FontNode (i, optional(), "Id"))); } list > subtitle_nodes; BOOST_FOREACH (cxml::NodePtr const & i, xml->node_children ("Subtitle")) { - subtitle_nodes.push_back (shared_ptr (new SubtitleNode (i, 250, "Id"))); + subtitle_nodes.push_back (shared_ptr (new SubtitleNode (i, optional(), "Id"))); } parse_subtitles (xml, font_nodes, subtitle_nodes); diff --git a/src/subtitle_node.cc b/src/subtitle_node.cc index bd689aa2..44c32e26 100644 --- a/src/subtitle_node.cc +++ b/src/subtitle_node.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -45,7 +45,8 @@ using boost::shared_ptr; using boost::lexical_cast; using namespace dcp; -SubtitleNode::SubtitleNode (boost::shared_ptr node, int tcr, string font_id_attribute) +/** @param tcr Timecode rate for SMPTE, or empty for Interop */ +SubtitleNode::SubtitleNode (boost::shared_ptr node, optional tcr, string font_id_attribute) { in = Time (node->string_attribute ("TimeIn"), tcr); out = Time (node->string_attribute ("TimeOut"), tcr); @@ -65,7 +66,7 @@ SubtitleNode::SubtitleNode (boost::shared_ptr node, int tcr, s } Time -SubtitleNode::fade_time (shared_ptr node, string name, int tcr) +SubtitleNode::fade_time (shared_ptr node, string name, optional tcr) { string const u = node->optional_string_attribute (name).get_value_or (""); Time t; @@ -75,7 +76,7 @@ SubtitleNode::fade_time (shared_ptr node, string name, int tcr } else if (u.find (":") != string::npos) { t = Time (u, tcr); } else { - t = Time (0, 0, 0, lexical_cast (u), tcr); + t = Time (0, 0, 0, lexical_cast (u), tcr.get_value_or(250)); } if (t > Time (0, 0, 8, 0, 250)) { diff --git a/src/subtitle_node.h b/src/subtitle_node.h index 28e410e3..37d41f01 100644 --- a/src/subtitle_node.h +++ b/src/subtitle_node.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -52,7 +52,7 @@ class SubtitleNode { public: SubtitleNode () {} - SubtitleNode (boost::shared_ptr node, int tcr, std::string font_id_attribute); + SubtitleNode (boost::shared_ptr node, boost::optional tcr, std::string font_id_attribute); Time in; Time out; @@ -62,7 +62,7 @@ public: std::list > text_nodes; private: - Time fade_time (boost::shared_ptr, std::string name, int tcr); + Time fade_time (boost::shared_ptr, std::string name, boost::optional tcr); }; } diff --git a/src/text_node.cc b/src/text_node.cc index 03777c75..cfd0557f 100644 --- a/src/text_node.cc +++ b/src/text_node.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -51,7 +51,7 @@ using namespace dcp; * in this object's member variables. * @param node Node to read. */ -TextNode::TextNode (boost::shared_ptr node, int tcr, string font_id_attribute) +TextNode::TextNode (boost::shared_ptr node, optional tcr, string font_id_attribute) : h_position (0) , h_align (HALIGN_CENTER) , v_position (0) diff --git a/src/text_node.h b/src/text_node.h index 8624a178..313bdbcb 100644 --- a/src/text_node.h +++ b/src/text_node.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2015 Carl Hetherington + Copyright (C) 2012-2016 Carl Hetherington This file is part of libdcp. @@ -63,7 +63,7 @@ public: , direction (DIRECTION_LTR) {} - TextNode (boost::shared_ptr node, int tcr, std::string font_id_attribute); + TextNode (boost::shared_ptr node, boost::optional tcr, std::string font_id_attribute); float h_position; HAlign h_align; diff --git a/test/dcp_time_test.cc b/test/dcp_time_test.cc index 9cd253c1..aff883ad 100644 --- a/test/dcp_time_test.cc +++ b/test/dcp_time_test.cc @@ -19,6 +19,9 @@ #include #include "dcp_time.h" +#include "exceptions.h" + +using boost::optional; /** Check that dcp::Time works */ BOOST_AUTO_TEST_CASE (dcp_time) @@ -100,4 +103,36 @@ BOOST_AUTO_TEST_CASE (dcp_time) /* We must round down in rebase() */ a = dcp::Time (0, 2, 57, 999, 1000); BOOST_CHECK_EQUAL (a.rebase (250), dcp::Time (0, 2, 57, 249, 250)); + + /* Check some allowed constructions from string */ + + /* Interop type 1 */ + a = dcp::Time ("01:23:45:123", optional()); + BOOST_CHECK_EQUAL (a, dcp::Time (1, 23, 45, 123, 250)); + /* Interop type 2 */ + a = dcp::Time ("01:23:45.123", optional()); + BOOST_CHECK_EQUAL (a, dcp::Time (1, 23, 45, 123, 1000)); + /* SMPTE */ + a = dcp::Time ("01:23:45:12", 250); + BOOST_CHECK_EQUAL (a, dcp::Time (1, 23, 45, 12, 250)); + + /* Check some disallowed constructions from string */ + BOOST_CHECK_THROW (dcp::Time ("01:23:45:1234", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45:1234:66", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45:", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23::123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01::45:123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time (":23:45:123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45.1234", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45.1234.66", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45.", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:.123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01::45.123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time (":23:45.123", optional()), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45:123", 250), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45:123:66", 250), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23:45:", 250), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01:23::123", 250), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time ("01::45:123", 250), dcp::DCPReadError); + BOOST_CHECK_THROW (dcp::Time (":23:45:123", 250), dcp::DCPReadError); } -- cgit v1.2.3