Fix thinko in relative path change.

We have to canonicalise relative paths with respect to the film's
directory on load, otherwise we try to use the relative path and it's
interpreted against the current working directory.

This unfortunately requires the film's directory to be piped into quite
a lot of new places.

21 files changed, 57 insertions, 64 deletions

diff --git a/src/lib/atmos_mxf_content.cc b/src/lib/atmos_mxf_content.cc
index 55ceddb8c..403f98933 100644
--- a/src/lib/atmos_mxf_content.cc
+++ b/src/lib/atmos_mxf_content.cc
@@ -48,8 +48,8 @@ AtmosMXFContent::AtmosMXFContent (boost::filesystem::path path)
 }
 
 
-AtmosMXFContent::AtmosMXFContent (cxml::ConstNodePtr node, int)
-	: Content (node)
+AtmosMXFContent::AtmosMXFContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int)
+	: Content(node, film_directory)
 {
 	atmos = AtmosContent::from_xml (this, node);
 }
diff --git a/src/lib/atmos_mxf_content.h b/src/lib/atmos_mxf_content.h
index 4d2d9f509..b0bdeb92f 100644
--- a/src/lib/atmos_mxf_content.h
+++ b/src/lib/atmos_mxf_content.h
@@ -27,7 +27,7 @@ class AtmosMXFContent : public Content
 {
 public:
 	AtmosMXFContent (boost::filesystem::path path);
-	AtmosMXFContent (cxml::ConstNodePtr node, int version);
+	AtmosMXFContent (cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version);
 
 	std::shared_ptr<AtmosMXFContent> shared_from_this () {
 		return std::dynamic_pointer_cast<AtmosMXFContent> (Content::shared_from_this());
diff --git a/src/lib/content.cc b/src/lib/content.cc
index 067a4cffc..be30ccf43 100644
--- a/src/lib/content.cc
+++ b/src/lib/content.cc
@@ -84,10 +84,14 @@ Content::Content (boost::filesystem::path p)
 }
 
 
-Content::Content (cxml::ConstNodePtr node)
+Content::Content(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory)
 {
 	for (auto i: node->node_children("Path")) {
-		_paths.push_back (i->content());
+		if (film_directory) {
+			_paths.push_back(boost::filesystem::weakly_canonical(boost::filesystem::absolute(i->content(), *film_directory)));
+		} else {
+			_paths.push_back(i->content());
+		}
 		auto const mod = i->optional_number_attribute<time_t>("mtime");
 		if (mod) {
 			_last_write_times.push_back (*mod);
@@ -147,19 +151,9 @@ Content::as_xml(xmlpp::Element* element, bool with_paths, PathBehaviour path_beh
 	if (with_paths) {
 		for (size_t i = 0; i < _paths.size(); ++i) {
 			auto path = _paths[i];
-			switch (path_behaviour) {
-			case PathBehaviour::MAKE_ABSOLUTE:
-				DCPOMATIC_ASSERT(film_directory);
-				if (path.is_relative()) {
-					path = boost::filesystem::canonical(path, *film_directory);
-				}
-				break;
-			case PathBehaviour::MAKE_RELATIVE:
+			if (path_behaviour == PathBehaviour::MAKE_RELATIVE) {
 				DCPOMATIC_ASSERT(film_directory);
 				path = boost::filesystem::relative(path, *film_directory);
-				break;
-			case PathBehaviour::KEEP:
-				break;
 			}
 			auto p = cxml::add_child(element, "Path");
 			p->add_child_text(path.string());
@@ -300,16 +294,16 @@ Content::set_trim_end (ContentTime t)
 
 
 shared_ptr<Content>
-Content::clone () const
+Content::clone() const
 {
 	/* This is a bit naughty, but I can't think of a compelling reason not to do it ... */
 	xmlpp::Document doc;
 	auto node = doc.create_root_node ("Content");
-	as_xml(node, true, PathBehaviour::KEEP, {});
+	as_xml(node, true, PathBehaviour::KEEP_ABSOLUTE, {});
 
 	/* notes is unused here (we assume) */
 	list<string> notes;
-	return content_factory (make_shared<cxml::Node>(node), Film::current_state_version, notes);
+	return content_factory(make_shared<cxml::Node>(node), {}, Film::current_state_version, notes);
 }
 
 
diff --git a/src/lib/content.h b/src/lib/content.h
index a14c35774..29dce2c9e 100644
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -76,7 +76,7 @@ public:
 	explicit Content ();
 	Content (dcpomatic::DCPTime);
 	Content (boost::filesystem::path);
-	Content (cxml::ConstNodePtr);
+	Content (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> directory);
 	Content (std::vector<std::shared_ptr<Content>>);
 	virtual ~Content () {}
 
diff --git a/src/lib/content_factory.cc b/src/lib/content_factory.cc
index bac41f940..5d36c2aea 100644
--- a/src/lib/content_factory.cc
+++ b/src/lib/content_factory.cc
@@ -48,7 +48,6 @@
 
 
 using std::list;
-using std::make_shared;
 using std::shared_ptr;
 using std::string;
 using std::vector;
@@ -56,29 +55,30 @@ using std::vector;
 
 /** Create a Content object from an XML node.
  *  @param node XML description.
+ *  @param directory "Current" directory for any relative file paths mentioned in the XML.
  *  @param version XML state version.
  *  @param notes A list to which is added descriptions of any non-critial warnings / messages.
  *  @return Content object, or 0 if no content was recognised in the XML.
  */
 shared_ptr<Content>
-content_factory (cxml::ConstNodePtr node, int version, list<string>& notes)
+content_factory(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version, list<string>& notes)
 {
 	auto const type = node->string_child ("Type");
 
 	std::shared_ptr<Content> content;
 
 	if (type == "FFmpeg") {
-		content = make_shared<FFmpegContent>(node, version, notes);
+		content = std::make_shared<FFmpegContent>(node, film_directory, version, notes);
 	} else if (type == "Image") {
-		content = make_shared<ImageContent>(node, version);
+		content = std::make_shared<ImageContent>(node, film_directory, version);
 	} else if (type == "Sndfile") {
 		/* SndfileContent is now handled by the FFmpeg code rather than by
 		   separate libsndfile-based code.
 		*/
-		content = make_shared<FFmpegContent>(node, version, notes);
+		content = std::make_shared<FFmpegContent>(node, film_directory, version, notes);
 
 		content->audio->set_stream (
-			make_shared<FFmpegAudioStream>(
+			std::make_shared<FFmpegAudioStream>(
 				"Stream", 0,
 				node->number_child<int> ("AudioFrameRate"),
 				node->number_child<Frame> ("AudioLength"),
@@ -88,15 +88,15 @@ content_factory (cxml::ConstNodePtr node, int version, list<string>& notes)
 			);
 
 	} else if (type == "SubRip" || type == "TextSubtitle") {
-		content = make_shared<StringTextFileContent>(node, version, notes);
+		content = std::make_shared<StringTextFileContent>(node, film_directory, version, notes);
 	} else if (type == "DCP") {
-		content = make_shared<DCPContent>(node, version);
+		content = std::make_shared<DCPContent>(node, film_directory, version);
 	} else if (type == "DCPSubtitle") {
-		content = make_shared<DCPSubtitleContent>(node, version);
+		content = std::make_shared<DCPSubtitleContent>(node, film_directory, version);
 	} else if (type == "VideoMXF") {
-		content = make_shared<VideoMXFContent>(node, version);
+		content = std::make_shared<VideoMXFContent>(node, film_directory, version);
 	} else if (type == "AtmosMXF") {
-		content = make_shared<AtmosMXFContent>(node, version);
+		content = std::make_shared<AtmosMXFContent>(node, film_directory, version);
 	}
 
 	return content;
@@ -153,10 +153,10 @@ content_factory (boost::filesystem::path path)
 		}
 
 		if (image_files > 0 && sound_files == 0)  {
-			content.push_back (make_shared<ImageContent>(path));
+			content.push_back(std::make_shared<ImageContent>(path));
 		} else if (image_files == 0 && sound_files > 0) {
 			for (auto i: dcp::filesystem::directory_iterator(path)) {
-				content.push_back (make_shared<FFmpegContent>(i.path()));
+				content.push_back(std::make_shared<FFmpegContent>(i.path()));
 			}
 		}
 
@@ -168,26 +168,26 @@ content_factory (boost::filesystem::path path)
 		transform (ext.begin(), ext.end(), ext.begin(), ::tolower);
 
 		if (valid_image_file (path)) {
-			single = make_shared<ImageContent>(path);
+			single = std::make_shared<ImageContent>(path);
 		} else if (ext == ".srt" || ext == ".ssa" || ext == ".ass" || ext == ".stl" || ext == ".vtt") {
-			single = make_shared<StringTextFileContent>(path);
+			single = std::make_shared<StringTextFileContent>(path);
 		} else if (ext == ".xml") {
 			cxml::Document doc;
 			doc.read_file(dcp::filesystem::fix_long_path(path));
 			if (doc.root_name() == "DCinemaSecurityMessage") {
 				throw KDMAsContentError ();
 			}
-			single = make_shared<DCPSubtitleContent>(path);
+			single = std::make_shared<DCPSubtitleContent>(path);
 		} else if (ext == ".mxf" && dcp::SMPTETextAsset::valid_mxf(path)) {
-			single = make_shared<DCPSubtitleContent>(path);
+			single = std::make_shared<DCPSubtitleContent>(path);
 		} else if (ext == ".mxf" && VideoMXFContent::valid_mxf(path)) {
-			single = make_shared<VideoMXFContent>(path);
+			single = std::make_shared<VideoMXFContent>(path);
 		} else if (ext == ".mxf" && AtmosMXFContent::valid_mxf(path)) {
-			single = make_shared<AtmosMXFContent>(path);
+			single = std::make_shared<AtmosMXFContent>(path);
 		}
 
 		if (!single) {
-			single = make_shared<FFmpegContent>(path);
+			single = std::make_shared<FFmpegContent>(path);
 		}
 
 		content.push_back (single);
diff --git a/src/lib/content_factory.h b/src/lib/content_factory.h
index 2cef84034..b4ce161fb 100644
--- a/src/lib/content_factory.h
+++ b/src/lib/content_factory.h
@@ -28,9 +28,8 @@
 #include <vector>
 
 
-class Film;
 class Content;
 
 
-extern std::shared_ptr<Content> content_factory (cxml::ConstNodePtr, int, std::list<std::string> &);
+extern std::shared_ptr<Content> content_factory(cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int, std::list<std::string> &);
 extern std::vector<std::shared_ptr<Content>> content_factory (boost::filesystem::path);
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 48d382c41..6a6ed0b0c 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -89,8 +89,8 @@ DCPContent::DCPContent (boost::filesystem::path p)
 	set_default_colour_conversion ();
 }
 
-DCPContent::DCPContent (cxml::ConstNodePtr node, int version)
-	: Content (node)
+DCPContent::DCPContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version)
+	: Content(node, film_directory)
 {
 	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 	audio = AudioContent::from_xml (this, node, version);
diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h
index 023b5d194..a866703f2 100644
--- a/src/lib/dcp_content.h
+++ b/src/lib/dcp_content.h
@@ -60,7 +60,7 @@ class DCPContent : public Content
 {
 public:
 	DCPContent (boost::filesystem::path p);
-	DCPContent (cxml::ConstNodePtr, int version);
+	DCPContent (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int version);
 
 	std::shared_ptr<DCPContent> shared_from_this () {
 		return std::dynamic_pointer_cast<DCPContent> (Content::shared_from_this ());
diff --git a/src/lib/dcp_subtitle_content.cc b/src/lib/dcp_subtitle_content.cc
index cbd8638f3..b18d14318 100644
--- a/src/lib/dcp_subtitle_content.cc
+++ b/src/lib/dcp_subtitle_content.cc
@@ -49,8 +49,8 @@ DCPSubtitleContent::DCPSubtitleContent (boost::filesystem::path path)
 	text.push_back (make_shared<TextContent>(this, TextType::OPEN_SUBTITLE, TextType::OPEN_SUBTITLE));
 }
 
-DCPSubtitleContent::DCPSubtitleContent (cxml::ConstNodePtr node, int version)
-	: Content (node)
+DCPSubtitleContent::DCPSubtitleContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version)
+	: Content (node, film_directory)
 	, _length (node->number_child<ContentTime::Type> ("Length"))
 {
 	list<string> notes;
diff --git a/src/lib/dcp_subtitle_content.h b/src/lib/dcp_subtitle_content.h
index 53b2eaf5c..14d36fb69 100644
--- a/src/lib/dcp_subtitle_content.h
+++ b/src/lib/dcp_subtitle_content.h
@@ -25,7 +25,7 @@ class DCPSubtitleContent : public DCPSubtitle, public Content
 {
 public:
 	DCPSubtitleContent (boost::filesystem::path);
-	DCPSubtitleContent (cxml::ConstNodePtr, int);
+	DCPSubtitleContent (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int);
 
 	void examine (std::shared_ptr<const Film> film, std::shared_ptr<Job>) override;
 	std::string summary () const override;
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 61a7dd4df..5a99ecaa5 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -88,8 +88,8 @@ get_optional_enum (cxml::ConstNodePtr node, string name)
 }
 
 
-FFmpegContent::FFmpegContent (cxml::ConstNodePtr node, int version, list<string>& notes)
-	: Content (node)
+FFmpegContent::FFmpegContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version, list<string>& notes)
+	: Content(node, film_directory)
 {
 	_color_range = get_optional_enum<AVColorRange>(node, "ColorRange");
 
diff --git a/src/lib/ffmpeg_content.h b/src/lib/ffmpeg_content.h
index 1b3d3376e..a0fe9a0c0 100644
--- a/src/lib/ffmpeg_content.h
+++ b/src/lib/ffmpeg_content.h
@@ -55,7 +55,7 @@ class FFmpegContent : public Content
 {
 public:
 	FFmpegContent (boost::filesystem::path);
-	FFmpegContent (cxml::ConstNodePtr, int version, std::list<std::string> &);
+	FFmpegContent (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int version, std::list<std::string> &);
 	FFmpegContent (std::vector<std::shared_ptr<Content>>);
 
 	std::shared_ptr<FFmpegContent> shared_from_this () {
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 835719868..ded96ab45 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -477,7 +477,7 @@ Film::metadata (bool with_content_paths) const
 	_playlist->as_xml(
 		cxml::add_child(root, "Playlist"),
 		with_content_paths,
-		Config::instance()->relative_paths() ? PathBehaviour::MAKE_RELATIVE : PathBehaviour::MAKE_ABSOLUTE,
+		Config::instance()->relative_paths() ? PathBehaviour::MAKE_RELATIVE : PathBehaviour::KEEP_ABSOLUTE,
 		directory()
 		);
 
diff --git a/src/lib/image_content.cc b/src/lib/image_content.cc
index 034ccb98d..e7ba88194 100644
--- a/src/lib/image_content.cc
+++ b/src/lib/image_content.cc
@@ -61,8 +61,8 @@ ImageContent::ImageContent (boost::filesystem::path p)
 }
 
 
-ImageContent::ImageContent (cxml::ConstNodePtr node, int version)
-	: Content (node)
+ImageContent::ImageContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version)
+	: Content(node, film_directory)
 {
 	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 }
diff --git a/src/lib/image_content.h b/src/lib/image_content.h
index 6208a0053..73ae696ad 100644
--- a/src/lib/image_content.h
+++ b/src/lib/image_content.h
@@ -27,7 +27,7 @@ class ImageContent : public Content
 {
 public:
 	ImageContent (boost::filesystem::path);
-	ImageContent (cxml::ConstNodePtr, int);
+	ImageContent (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int);
 
 	std::shared_ptr<ImageContent> shared_from_this () {
 		return std::dynamic_pointer_cast<ImageContent> (Content::shared_from_this ());
diff --git a/src/lib/path_behaviour.h b/src/lib/path_behaviour.h
index 90390e015..cf966af5a 100644
--- a/src/lib/path_behaviour.h
+++ b/src/lib/path_behaviour.h
@@ -24,9 +24,8 @@
 
 
 enum class PathBehaviour {
-	MAKE_ABSOLUTE,
+	KEEP_ABSOLUTE,
 	MAKE_RELATIVE,
-	KEEP
 };
 
 
diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc
index b6b796840..ebeac47e2 100644
--- a/src/lib/playlist.cc
+++ b/src/lib/playlist.cc
@@ -25,6 +25,7 @@
 #include "content_factory.h"
 #include "dcp_content.h"
 #include "digester.h"
+#include "film.h"
 #include "ffmpeg_content.h"
 #include "ffmpeg_decoder.h"
 #include "image_decoder.h"
@@ -226,7 +227,7 @@ Playlist::set_from_xml (shared_ptr<const Film> film, cxml::ConstNodePtr node, in
 	boost::mutex::scoped_lock lm (_mutex);
 
 	for (auto i: node->node_children ("Content")) {
-		auto content = content_factory (i, version, notes);
+		auto content = content_factory(i, film->directory(), version, notes);
 
 		/* See if this content should be nudged to start on a video frame */
 		auto const old_pos = content->position();
diff --git a/src/lib/string_text_file_content.cc b/src/lib/string_text_file_content.cc
index 6ae9fc627..4f7fbb241 100644
--- a/src/lib/string_text_file_content.cc
+++ b/src/lib/string_text_file_content.cc
@@ -52,8 +52,8 @@ StringTextFileContent::StringTextFileContent (boost::filesystem::path path)
 }
 
 
-StringTextFileContent::StringTextFileContent (cxml::ConstNodePtr node, int version, list<string>& notes)
-	: Content (node)
+StringTextFileContent::StringTextFileContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version, list<string>& notes)
+	: Content (node, film_directory)
 	, _length (node->number_child<ContentTime::Type>("Length"))
 {
 	text = TextContent::from_xml (this, node, version, notes);
diff --git a/src/lib/string_text_file_content.h b/src/lib/string_text_file_content.h
index 2edf475c3..abfd72906 100644
--- a/src/lib/string_text_file_content.h
+++ b/src/lib/string_text_file_content.h
@@ -32,7 +32,7 @@ class StringTextFileContent : public Content
 {
 public:
 	StringTextFileContent (boost::filesystem::path);
-	StringTextFileContent (cxml::ConstNodePtr, int, std::list<std::string>&);
+	StringTextFileContent (cxml::ConstNodePtr, boost::optional<boost::filesystem::path> film_directory, int, std::list<std::string>&);
 
 	std::shared_ptr<StringTextFileContent> shared_from_this () {
 		return std::dynamic_pointer_cast<StringTextFileContent> (Content::shared_from_this ());
diff --git a/src/lib/video_mxf_content.cc b/src/lib/video_mxf_content.cc
index 9ae439be5..ded81e6bf 100644
--- a/src/lib/video_mxf_content.cc
+++ b/src/lib/video_mxf_content.cc
@@ -49,8 +49,8 @@ VideoMXFContent::VideoMXFContent (boost::filesystem::path path)
 }
 
 
-VideoMXFContent::VideoMXFContent (cxml::ConstNodePtr node, int version)
-	: Content (node)
+VideoMXFContent::VideoMXFContent(cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version)
+	: Content(node, film_directory)
 {
 	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 }
diff --git a/src/lib/video_mxf_content.h b/src/lib/video_mxf_content.h
index d45ec591a..0def6008c 100644
--- a/src/lib/video_mxf_content.h
+++ b/src/lib/video_mxf_content.h
@@ -26,7 +26,7 @@ class VideoMXFContent : public Content
 {
 public:
 	VideoMXFContent (boost::filesystem::path path);
-	VideoMXFContent (cxml::ConstNodePtr node, int version);
+	VideoMXFContent (cxml::ConstNodePtr node, boost::optional<boost::filesystem::path> film_directory, int version);
 
 	std::shared_ptr<VideoMXFContent> shared_from_this () {
 		return std::dynamic_pointer_cast<VideoMXFContent>(Content::shared_from_this());