Handle video frame rates and subtitles a bit more correctly.

10 files changed, 134 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index 37344427c..bdd295ddc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-09-30  Carl Hetherington  <cth@carlh.net>
+
+	* Fix various confusions with subtitles and
+	changing video frame rates.
+
 2015-09-29  Carl Hetherington  <cth@carlh.net>
 
 	* Version 2.3.10 released.
diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h
index 8f01bbc4e..0f59bdf43 100644
--- a/src/lib/dcp_content.h
+++ b/src/lib/dcp_content.h
@@ -73,6 +73,10 @@ public:
 		return false;
 	}
 
+	double subtitle_video_frame_rate () const {
+		return video_frame_rate ();
+	}
+
 	boost::filesystem::path directory () const;
 
 	bool encrypted () const {
diff --git a/src/lib/dcp_subtitle_content.cc b/src/lib/dcp_subtitle_content.cc
index 8e24cb4c6..440b10a19 100644
--- a/src/lib/dcp_subtitle_content.cc
+++ b/src/lib/dcp_subtitle_content.cc
@@ -20,6 +20,7 @@
 #include "font.h"
 #include "dcp_subtitle_content.h"
 #include "raw_convert.h"
+#include "film.h"
 #include <dcp/interop_subtitle_asset.h>
 #include <dcp/smpte_subtitle_asset.h>
 #include <dcp/interop_load_font_node.h>
@@ -43,7 +44,8 @@ DCPSubtitleContent::DCPSubtitleContent (shared_ptr<const Film> film, boost::file
 DCPSubtitleContent::DCPSubtitleContent (shared_ptr<const Film> film, cxml::ConstNodePtr node, int version)
 	: Content (film, node)
 	, SubtitleContent (film, node, version)
-	, _length (node->number_child<DCPTime::Type> ("Length"))
+	, _length (node->number_child<ContentTime::Type> ("Length"))
+	, _frame_rate (node->optional_number_child<int>("SubtitleFrameRate"))
 {
 
 }
@@ -67,9 +69,10 @@ DCPSubtitleContent::examine (shared_ptr<Job> job)
 	shared_ptr<dcp::SMPTESubtitleAsset> smpte = dynamic_pointer_cast<dcp::SMPTESubtitleAsset> (sc);
 	if (smpte) {
 		_subtitle_language = smpte->language().get_value_or ("");
+		_frame_rate = smpte->edit_rate().numerator;
 	}
 
-	_length = DCPTime::from_seconds (sc->latest_subtitle_out().as_seconds ());
+	_length = ContentTime::from_seconds (sc->latest_subtitle_out().as_seconds ());
 
 	BOOST_FOREACH (shared_ptr<dcp::LoadFontNode> i, sc->load_font_nodes ()) {
 		add_font (shared_ptr<Font> (new Font (i->id)));
@@ -79,10 +82,10 @@ DCPSubtitleContent::examine (shared_ptr<Job> job)
 DCPTime
 DCPSubtitleContent::full_length () const
 {
-	/* XXX: this assumes that the timing of the subtitle file is appropriate
-	   for the DCP's frame rate.
-	*/
-	return _length;
+	shared_ptr<const Film> film = _film.lock ();
+	DCPOMATIC_ASSERT (film);
+	FrameRateChange const frc (subtitle_video_frame_rate(), film->video_frame_rate());
+	return DCPTime (_length, frc);
 }
 
 string
@@ -105,3 +108,30 @@ DCPSubtitleContent::as_xml (xmlpp::Node* node) const
 	SubtitleContent::as_xml (node);
 	node->add_child("Length")->add_child_text (raw_convert<string> (_length.get ()));
 }
+
+void
+DCPSubtitleContent::set_subtitle_video_frame_rate (int r)
+{
+	{
+		boost::mutex::scoped_lock lm (_mutex);
+		_frame_rate = r;
+	}
+
+	signal_changed (SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE);
+}
+
+double
+DCPSubtitleContent::subtitle_video_frame_rate () const
+{
+	boost::mutex::scoped_lock lm (_mutex);
+	if (_frame_rate) {
+		return _frame_rate.get ();
+	}
+
+	/* No frame rate specified, so assume this content has been
+	   prepared for any concurrent video content.
+	*/
+	shared_ptr<const Film> film = _film.lock ();
+	DCPOMATIC_ASSERT (film);
+	return film->active_frame_rate_change(position()).source;
+}
diff --git a/src/lib/dcp_subtitle_content.h b/src/lib/dcp_subtitle_content.h
index 9fea81c2a..887328393 100644
--- a/src/lib/dcp_subtitle_content.h
+++ b/src/lib/dcp_subtitle_content.h
@@ -43,6 +43,11 @@ public:
 		return false;
 	}
 
+	double subtitle_video_frame_rate () const;
+	void set_subtitle_video_frame_rate (int r);
+
 private:
-	DCPTime _length;
+	ContentTime _length;
+	/** Video frame rate that this content has been prepared for, if known */
+	boost::optional<double> _frame_rate;
 };
diff --git a/src/lib/ffmpeg_content.h b/src/lib/ffmpeg_content.h
index 3159abfd1..5437c5bf3 100644
--- a/src/lib/ffmpeg_content.h
+++ b/src/lib/ffmpeg_content.h
@@ -69,6 +69,9 @@ public:
 	/* SubtitleContent */
 	bool has_text_subtitles () const;
 	bool has_image_subtitles () const;
+	double subtitle_video_frame_rate () const {
+		return video_frame_rate ();
+	}
 
 	void set_filters (std::vector<Filter const *> const &);
 
diff --git a/src/lib/subrip_content.cc b/src/lib/subrip_content.cc
index 0d1bd748d..ba081624a 100644
--- a/src/lib/subrip_content.cc
+++ b/src/lib/subrip_content.cc
@@ -45,7 +45,8 @@ SubRipContent::SubRipContent (shared_ptr<const Film> film, boost::filesystem::pa
 SubRipContent::SubRipContent (shared_ptr<const Film> film, cxml::ConstNodePtr node, int version)
 	: Content (film, node)
 	, SubtitleContent (film, node, version)
-	, _length (node->number_child<DCPTime::Type> ("Length"))
+	, _length (node->number_child<ContentTime::Type> ("Length"))
+	, _frame_rate (node->optional_number_child<double>("SubtitleFrameRate"))
 {
 
 }
@@ -56,16 +57,11 @@ SubRipContent::examine (boost::shared_ptr<Job> job)
 	Content::examine (job);
 	SubRip s (shared_from_this ());
 
-	shared_ptr<const Film> film = _film.lock ();
-	DCPOMATIC_ASSERT (film);
-
-	DCPTime len (s.length (), film->active_frame_rate_change (position ()));
-
 	/* Default to turning these subtitles on */
 	set_use_subtitles (true);
 
 	boost::mutex::scoped_lock lm (_mutex);
-	_length = len;
+	_length = s.length ();
 	add_font (shared_ptr<Font> (new Font (font_id)));
 }
 
@@ -93,8 +89,37 @@ SubRipContent::as_xml (xmlpp::Node* node) const
 DCPTime
 SubRipContent::full_length () const
 {
-	/* XXX: this assumes that the timing of the SubRip file is appropriate
-	   for the DCP's frame rate.
+	shared_ptr<const Film> film = _film.lock ();
+	DCPOMATIC_ASSERT (film);
+	FrameRateChange const frc (subtitle_video_frame_rate(), film->video_frame_rate ());
+	return DCPTime (_length, frc);
+}
+
+void
+SubRipContent::set_subtitle_video_frame_rate (int r)
+{
+	{
+		boost::mutex::scoped_lock lm (_mutex);
+		_frame_rate = r;
+	}
+
+	signal_changed (SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE);
+}
+
+double
+SubRipContent::subtitle_video_frame_rate () const
+{
+	{
+		boost::mutex::scoped_lock lm (_mutex);
+		if (_frame_rate) {
+			return _frame_rate.get ();
+		}
+	}
+
+	/* No frame rate specified, so assume this content has been
+	   prepared for any concurrent video content.
 	*/
-	return _length;
+	shared_ptr<const Film> film = _film.lock ();
+	DCPOMATIC_ASSERT (film);
+	return film->active_frame_rate_change(position()).source;
 }
diff --git a/src/lib/subrip_content.h b/src/lib/subrip_content.h
index 342efd4de..e05063403 100644
--- a/src/lib/subrip_content.h
+++ b/src/lib/subrip_content.h
@@ -46,8 +46,13 @@ public:
 		return false;
 	}
 
+	double subtitle_video_frame_rate () const;
+	void set_subtitle_video_frame_rate (int r);
+
 	static std::string const font_id;
 
 private:
-	DCPTime _length;
+	ContentTime _length;
+	/** Video frame rate that this content has been prepared for, if known */
+	boost::optional<double> _frame_rate;
 };
diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc
index da3b97e2f..2f4c88975 100644
--- a/src/lib/subtitle_content.cc
+++ b/src/lib/subtitle_content.cc
@@ -45,6 +45,7 @@ int const SubtitleContentProperty::USE_SUBTITLES = 504;
 int const SubtitleContentProperty::BURN_SUBTITLES = 505;
 int const SubtitleContentProperty::SUBTITLE_LANGUAGE = 506;
 int const SubtitleContentProperty::FONTS = 507;
+int const SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE = 508;
 
 SubtitleContent::SubtitleContent (shared_ptr<const Film> film)
 	: Content (film)
diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h
index 99cf3b382..e9017e40d 100644
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@ -35,6 +35,7 @@ public:
 	static int const BURN_SUBTITLES;
 	static int const SUBTITLE_LANGUAGE;
 	static int const FONTS;
+	static int const SUBTITLE_VIDEO_FRAME_RATE;
 };
 
 /** @class SubtitleContent
@@ -57,6 +58,7 @@ public:
 	bool has_subtitles () const;
 	virtual bool has_text_subtitles () const = 0;
 	virtual bool has_image_subtitles () const = 0;
+	virtual double subtitle_video_frame_rate () const = 0;
 
 	void add_font (boost::shared_ptr<Font> font);
 
diff --git a/src/wx/timing_panel.cc b/src/wx/timing_panel.cc
index 6ebd83d73..5d74773b2 100644
--- a/src/wx/timing_panel.cc
+++ b/src/wx/timing_panel.cc
@@ -17,14 +17,17 @@
 
 */
 
-#include "lib/content.h"
-#include "lib/image_content.h"
-#include "lib/raw_convert.h"
 #include "timing_panel.h"
 #include "wx_util.h"
 #include "film_viewer.h"
 #include "timecode.h"
 #include "content_panel.h"
+#include "lib/content.h"
+#include "lib/image_content.h"
+#include "lib/raw_convert.h"
+#include "lib/subtitle_content.h"
+#include "lib/dcp_subtitle_content.h"
+#include "lib/subrip_content.h"
 #include <boost/foreach.hpp>
 #include <set>
 #include <iostream>
@@ -200,7 +203,8 @@ TimingPanel::film_content_changed (int property)
 	} else if (
 		property == ContentProperty::LENGTH ||
 		property == VideoContentProperty::VIDEO_FRAME_RATE ||
-		property == VideoContentProperty::VIDEO_FRAME_TYPE
+		property == VideoContentProperty::VIDEO_FRAME_TYPE ||
+		property == SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE
 		) {
 
 		update_full_length ();
@@ -237,7 +241,8 @@ TimingPanel::film_content_changed (int property)
 		property == ContentProperty::TRIM_START ||
 		property == ContentProperty::TRIM_END ||
 		property == VideoContentProperty::VIDEO_FRAME_RATE ||
-		property == VideoContentProperty::VIDEO_FRAME_TYPE
+		property == VideoContentProperty::VIDEO_FRAME_TYPE ||
+		property == SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE
 		) {
 
 		update_play_length ();
@@ -262,6 +267,25 @@ TimingPanel::film_content_changed (int property)
 		}
 	}
 
+	if (property == SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE) {
+		shared_ptr<const SubtitleContent> check;
+		int count = 0;
+		BOOST_FOREACH (shared_ptr<const Content> i, _parent->selected ()) {
+			shared_ptr<const SubtitleContent> t = dynamic_pointer_cast<const SubtitleContent> (i);
+			if (t) {
+				check = t;
+				++count;
+			}
+		}
+		if (count == 1) {
+			checked_set (_video_frame_rate, raw_convert<string> (check->subtitle_video_frame_rate (), 5));
+			_video_frame_rate->Enable (true);
+		} else {
+			checked_set (_video_frame_rate, wxT (""));
+			_video_frame_rate->Enable (false);
+		}
+	}
+
 	bool have_still = false;
 	BOOST_FOREACH (shared_ptr<const Content> i, _parent->selected ()) {
 		shared_ptr<const ImageContent> ic = dynamic_pointer_cast<const ImageContent> (i);
@@ -335,8 +359,14 @@ TimingPanel::set_video_frame_rate ()
 {
 	BOOST_FOREACH (shared_ptr<Content> i, _parent->selected ()) {
 		shared_ptr<VideoContent> vc = dynamic_pointer_cast<VideoContent> (i);
+		shared_ptr<DCPSubtitleContent> dsc = dynamic_pointer_cast<DCPSubtitleContent> (i);
+		shared_ptr<SubRipContent> ssc = dynamic_pointer_cast<SubRipContent> (i);
 		if (vc) {
 			vc->set_video_frame_rate (raw_convert<double> (wx_to_std (_video_frame_rate->GetValue ())));
+		} else if (dsc) {
+			dsc->set_subtitle_video_frame_rate (raw_convert<double> (wx_to_std (_video_frame_rate->GetValue ())));
+		} else if (ssc) {
+			ssc->set_subtitle_video_frame_rate (raw_convert<double> (wx_to_std (_video_frame_rate->GetValue ())));
 		}
 		_set_video_frame_rate->Enable (false);
 	}
@@ -359,6 +389,7 @@ TimingPanel::content_selection_changed ()
 	film_content_changed (ContentProperty::TRIM_START);
 	film_content_changed (ContentProperty::TRIM_END);
 	film_content_changed (VideoContentProperty::VIDEO_FRAME_RATE);
+	film_content_changed (SubtitleContentProperty::SUBTITLE_VIDEO_FRAME_RATE);
 }
 
 void