From: Carl Hetherington <cth@carlh.net>
Date: Thu, 12 Jun 2014 22:34:31 +0000 (+0100)
Subject: Fix subrip subtitles a little.
X-Git-Tag: v2.0.48~798
X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=41d32a43f761fd2d79dacf9a84374a6d17916d36;hp=141f9a795381fa3bbd3f2bcbd19975dfd8a8c35e

Fix subrip subtitles a little.
---

diff --git a/src/lib/dcpomatic_time.cc b/src/lib/dcpomatic_time.cc
index 98888646d..ae4dea44f 100644
--- a/src/lib/dcpomatic_time.cc
+++ b/src/lib/dcpomatic_time.cc
@@ -49,3 +49,9 @@ operator<< (ostream& s, DCPTime t)
 	s << "[DCP " << t.get() << " " << t.seconds() << "s]";
 	return s;
 }
+
+bool
+ContentTimePeriod::overlaps (ContentTimePeriod const & other) const
+{
+	return (from < other.to && to > other.from);
+}
diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h
index 2a871889a..142104073 100644
--- a/src/lib/dcpomatic_time.h
+++ b/src/lib/dcpomatic_time.h
@@ -161,6 +161,8 @@ public:
 	ContentTimePeriod operator+ (ContentTime const & o) const {
 		return ContentTimePeriod (from + o, to + o);
 	}
+
+	bool overlaps (ContentTimePeriod const & o) const;
 };
 
 class DCPTime : public Time
diff --git a/src/lib/ffmpeg_content.h b/src/lib/ffmpeg_content.h
index 1a30fb606..367c30103 100644
--- a/src/lib/ffmpeg_content.h
+++ b/src/lib/ffmpeg_content.h
@@ -73,9 +73,6 @@ public:
 	void set_audio_mapping (AudioMapping);
 	boost::filesystem::path audio_analysis_path () const;
 
-	/* SubtitleContent */
-	bool has_subtitle_during (ContentTimePeriod) const;
-
 	void set_filters (std::vector<Filter const *> const &);
 	
 	std::vector<boost::shared_ptr<FFmpegSubtitleStream> > subtitle_streams () const {
@@ -111,6 +108,8 @@ public:
 		return _first_video;
 	}
 
+	bool has_subtitle_during (ContentTimePeriod) const;
+
 private:
 	friend class ffmpeg_pts_offset_test;
 	
diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index a2b71aec3..2041a4d17 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -575,3 +575,9 @@ FFmpegDecoder::decode_subtitle_packet ()
 	
 	avsubtitle_free (&sub);
 }
+
+bool
+FFmpegDecoder::has_subtitle_during (ContentTimePeriod p) const
+{
+	return _ffmpeg_content->has_subtitle_during (p);
+}
diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h
index 6006fc08d..335364d2e 100644
--- a/src/lib/ffmpeg_decoder.h
+++ b/src/lib/ffmpeg_decoder.h
@@ -72,6 +72,8 @@ private:
 	int minimal_run (boost::function<bool (boost::optional<ContentTime>, boost::optional<ContentTime>, int)>);
 	void seek_and_flush (ContentTime);
 
+	bool has_subtitle_during (ContentTimePeriod) const;
+	
 	boost::shared_ptr<Log> _log;
 	AVCodecContext* _subtitle_codec_context; ///< may be 0 if there is no subtitle
 	AVCodec* _subtitle_codec;		 ///< may be 0 if there is no subtitle
diff --git a/src/lib/subrip.cc b/src/lib/subrip.cc
index 9d207d528..3217a4df0 100644
--- a/src/lib/subrip.cc
+++ b/src/lib/subrip.cc
@@ -90,8 +90,7 @@ SubRip::SubRip (shared_ptr<const SubRipContent> content)
 				throw SubRipError (line, _("a time/position line"), content->path (0));
 			}
 
-			current->from = convert_time (p[0]);
-			current->to = convert_time (p[2]);
+			current->period = ContentTimePeriod (convert_time (p[0]), convert_time (p[2]));
 
 			if (p.size() > 3) {
 				current->x1 = convert_coordinate (p[3]);
@@ -233,5 +232,5 @@ SubRip::length () const
 		return ContentTime ();
 	}
 
-	return _subtitles.back().to;
+	return _subtitles.back().period.to;
 }
diff --git a/src/lib/subrip_content.cc b/src/lib/subrip_content.cc
index d7825f518..892578ade 100644
--- a/src/lib/subrip_content.cc
+++ b/src/lib/subrip_content.cc
@@ -109,10 +109,3 @@ SubRipContent::identifier () const
 
 	return s.str ();
 }
-
-bool
-SubRipContent::has_subtitle_during (ContentTimePeriod) const
-{
-	/* XXX */
-	return false;
-}
diff --git a/src/lib/subrip_content.h b/src/lib/subrip_content.h
index 91de08350..5688f81d5 100644
--- a/src/lib/subrip_content.h
+++ b/src/lib/subrip_content.h
@@ -37,8 +37,6 @@ public:
 	DCPTime full_length () const;
 	std::string identifier () const;
 
-	bool has_subtitle_during (ContentTimePeriod) const;
-
 private:
 	DCPTime _length;
 };
diff --git a/src/lib/subrip_decoder.cc b/src/lib/subrip_decoder.cc
index e832c2d84..3d971fd4b 100644
--- a/src/lib/subrip_decoder.cc
+++ b/src/lib/subrip_decoder.cc
@@ -22,6 +22,7 @@
 #include "subrip_content.h"
 
 using std::list;
+using std::vector;
 using boost::shared_ptr;
 
 SubRipDecoder::SubRipDecoder (shared_ptr<const SubRipContent> content)
@@ -39,7 +40,7 @@ SubRipDecoder::seek (ContentTime time, bool accurate)
 	
 	_next = 0;
 	list<SubRipSubtitlePiece>::const_iterator i = _subtitles[_next].pieces.begin();
-	while (i != _subtitles[_next].pieces.end() && _subtitles[_next].from < time) {
+	while (i != _subtitles[_next].pieces.end() && _subtitles[_next].period.from < time) {
 		++i;
 	}
 	
@@ -60,8 +61,8 @@ SubRipDecoder::pass ()
 				i->italic,
 				dcp::Color (255, 255, 255),
 				72,
-				dcp::Time (rint (_subtitles[_next].from.seconds() * 250)),
-				dcp::Time (rint (_subtitles[_next].to.seconds() * 250)),
+				dcp::Time (rint (_subtitles[_next].period.from.seconds() * 250)),
+				dcp::Time (rint (_subtitles[_next].period.to.seconds() * 250)),
 				0.9,
 				dcp::BOTTOM,
 				i->text,
@@ -77,3 +78,17 @@ SubRipDecoder::pass ()
 	_next++;
 	return false;
 }
+
+bool
+SubRipDecoder::has_subtitle_during (ContentTimePeriod p) const
+{
+	/* XXX: inefficient */
+
+	for (vector<SubRipSubtitle>::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+		if (p.overlaps (i->period)) {
+			return true;
+		}
+	}
+
+	return false;
+}
diff --git a/src/lib/subrip_decoder.h b/src/lib/subrip_decoder.h
index ca885a2ef..d6c6e6bc2 100644
--- a/src/lib/subrip_decoder.h
+++ b/src/lib/subrip_decoder.h
@@ -35,6 +35,8 @@ protected:
 	bool pass ();
 
 private:
+	bool has_subtitle_during (ContentTimePeriod) const;
+	
 	size_t _next;
 };
 
diff --git a/src/lib/subrip_subtitle.h b/src/lib/subrip_subtitle.h
index dd46b6c64..646fc1f7a 100644
--- a/src/lib/subrip_subtitle.h
+++ b/src/lib/subrip_subtitle.h
@@ -42,13 +42,7 @@ struct SubRipSubtitlePiece
 
 struct SubRipSubtitle
 {
-	SubRipSubtitle ()
-		: from (0)
-		, to (0)
-	{}
-	
-	ContentTime from;
-	ContentTime to;
+	ContentTimePeriod period;
 	boost::optional<int> x1;
 	boost::optional<int> x2;
 	boost::optional<int> y1;
diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h
index cc91a2df8..7d4a385c9 100644
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@ -39,8 +39,6 @@ public:
 
 	void as_xml (xmlpp::Node *) const;
 
-	virtual bool has_subtitle_during (ContentTimePeriod) const = 0;
-	
 	void set_subtitle_x_offset (double);
 	void set_subtitle_y_offset (double);
 	void set_subtitle_scale (double);
diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc
index 13cf481c8..fc03442d5 100644
--- a/src/lib/subtitle_decoder.cc
+++ b/src/lib/subtitle_decoder.cc
@@ -51,7 +51,8 @@ template <class T>
 list<shared_ptr<T> >
 SubtitleDecoder::get (list<shared_ptr<T> > const & subs, ContentTimePeriod period)
 {
-	if (!_subtitle_content->has_subtitle_during (period)) {
+	if (!has_subtitle_during (period)) {
+		cout << "no subtitle during this period.\n";
 		return list<shared_ptr<T> > ();
 	}
 
@@ -71,7 +72,7 @@ SubtitleDecoder::get (list<shared_ptr<T> > const & subs, ContentTimePeriod perio
 	
 	list<shared_ptr<T> > out;
 	for (typename list<shared_ptr<T> >::const_iterator i = subs.begin(); i != subs.end(); ++i) {
-		if ((*i)->period().from <= period.to && (*i)->period().to >= period.from) {
+		if ((*i)->period().overlaps (period)) {
 			out.push_back (*i);
 		}
 	}
diff --git a/src/lib/subtitle_decoder.h b/src/lib/subtitle_decoder.h
index 164c151e6..c25edad49 100644
--- a/src/lib/subtitle_decoder.h
+++ b/src/lib/subtitle_decoder.h
@@ -51,6 +51,8 @@ private:
 	template <class T>
 	std::list<boost::shared_ptr<T> > get (std::list<boost::shared_ptr<T> > const & subs, ContentTimePeriod period);
 
+	virtual bool has_subtitle_during (ContentTimePeriod) const = 0;
+	
 	boost::shared_ptr<const SubtitleContent> _subtitle_content;
 };
 
diff --git a/test/subrip_test.cc b/test/subrip_test.cc
index 19caca592..adb548f0c 100644
--- a/test/subrip_test.cc
+++ b/test/subrip_test.cc
@@ -135,45 +135,45 @@ BOOST_AUTO_TEST_CASE (subrip_parse_test)
 	vector<SubRipSubtitle>::const_iterator i = s._subtitles.begin();
 
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((1 * 60) + 49.200));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((1 * 60) + 52.351));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((1 * 60) + 49.200));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((1 * 60) + 52.351));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "This is a subtitle, and it goes over two lines.");
 
 	++i;
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((1 * 60) + 52.440));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((1 * 60) + 54.351));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((1 * 60) + 52.440));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((1 * 60) + 54.351));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "We have emboldened this");
 	BOOST_CHECK_EQUAL (i->pieces.front().bold, true);
 
 	++i;
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((1 * 60) + 54.440));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((1 * 60) + 56.590));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((1 * 60) + 54.440));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((1 * 60) + 56.590));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "And italicised this.");
 	BOOST_CHECK_EQUAL (i->pieces.front().italic, true);
 
 	++i;
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((1 * 60) + 56.680));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((1 * 60) + 58.955));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((1 * 60) + 56.680));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((1 * 60) + 58.955));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "Shall I compare thee to a summers' day?");
 
 	++i;
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((2 * 60) + 0.840));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((2 * 60) + 3.400));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((2 * 60) + 0.840));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((2 * 60) + 3.400));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "Is this a dagger I see before me?");
 
 	++i;
 	BOOST_CHECK (i != s._subtitles.end ());
-	BOOST_CHECK_EQUAL (i->from, ContentTime::from_seconds ((3 * 60) + 54.560));
-	BOOST_CHECK_EQUAL (i->to, ContentTime::from_seconds ((3 * 60) + 56.471));
+	BOOST_CHECK_EQUAL (i->period.from, ContentTime::from_seconds ((3 * 60) + 54.560));
+	BOOST_CHECK_EQUAL (i->period.to, ContentTime::from_seconds ((3 * 60) + 56.471));
 	BOOST_CHECK_EQUAL (i->pieces.size(), 1);
 	BOOST_CHECK_EQUAL (i->pieces.front().text, "Hello world.");