14 files changed, 162 insertions, 44 deletions

diff --git a/src/lib/analyse_audio_job.cc b/src/lib/analyse_audio_job.cc
index 2848c1ed7..9a9116690 100644
--- a/src/lib/analyse_audio_job.cc
+++ b/src/lib/analyse_audio_job.cc
@@ -68,14 +68,14 @@ AnalyseAudioJob::run ()
 	
 	player->Audio.connect (bind (&AnalyseAudioJob::audio, this, _1, _2));
 
-	_samples_per_point = max (int64_t (1), _film->time_to_audio_frames (_film->length()) / _num_points);
+	_samples_per_point = max (int64_t (1), _film->time_to_audio_frames (_film->length_without_loop()) / _num_points);
 
 	_current.resize (_film->dcp_audio_channels ());
 	_analysis.reset (new AudioAnalysis (_film->dcp_audio_channels ()));
 
 	_done = 0;
 	while (!player->pass ()) {
-		set_progress (double (_film->audio_frames_to_time (_done)) / _film->length ());
+		set_progress (double (_film->audio_frames_to_time (_done)) / _film->length_without_loop ());
 	}
 
 	_analysis->write (content->audio_analysis_path ());
diff --git a/src/lib/content.cc b/src/lib/content.cc
index b49ea4316..531dbc38f 100644
--- a/src/lib/content.cc
+++ b/src/lib/content.cc
@@ -22,6 +22,7 @@
 #include <libcxml/cxml.h>
 #include "content.h"
 #include "util.h"
+#include "content_factory.h"
 
 using std::string;
 using std::set;
@@ -90,3 +91,18 @@ Content::set_start (Time s)
 
 	signal_changed (ContentProperty::START);
 }
+
+shared_ptr<Content>
+Content::clone () const
+{
+	shared_ptr<const Film> film = _film.lock ();
+	if (!film) {
+		return shared_ptr<Content> ();
+	}
+	
+	/* This is a bit naughty, but I can't think of a compelling reason not to do it ... */
+	xmlpp::Document doc;
+	xmlpp::Node* node = doc.create_root_node ("Content");
+	as_xml (node);
+	return content_factory (film, shared_ptr<cxml::Node> (new cxml::Node (node)));
+}
diff --git a/src/lib/content.h b/src/lib/content.h
index a340fb1aa..26e115354 100644
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -56,6 +56,8 @@ public:
 	virtual void as_xml (xmlpp::Node *) const;
 	virtual Time length () const = 0;
 
+	boost::shared_ptr<Content> clone () const;
+	
 	boost::filesystem::path file () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _file;
diff --git a/src/lib/content_factory.cc b/src/lib/content_factory.cc
new file mode 100644
index 000000000..cf45b6aa6
--- /dev/null
+++ b/src/lib/content_factory.cc
@@ -0,0 +1,44 @@
+/*
+    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+
+    This program 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.
+
+    This program 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 this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#include <libcxml/cxml.h>
+#include "ffmpeg_content.h"
+#include "imagemagick_content.h"
+#include "sndfile_content.h"
+
+using std::string;
+using boost::shared_ptr;
+
+shared_ptr<Content>
+content_factory (shared_ptr<const Film> film, shared_ptr<cxml::Node> node)
+{
+	string const type = node->string_child ("Type");
+
+	boost::shared_ptr<Content> content;
+	
+	if (type == "FFmpeg") {
+		content.reset (new FFmpegContent (film, node));
+	} else if (type == "ImageMagick") {
+		content.reset (new ImageMagickContent (film, node));
+	} else if (type == "Sndfile") {
+		content.reset (new SndfileContent (film, node));
+	}
+
+	return content;
+}
diff --git a/src/lib/content_factory.h b/src/lib/content_factory.h
new file mode 100644
index 000000000..27cd36024
--- /dev/null
+++ b/src/lib/content_factory.h
@@ -0,0 +1,22 @@
+/*
+    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+
+    This program 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.
+
+    This program 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 this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+class Film;
+
+extern boost::shared_ptr<Content> content_factory (boost::shared_ptr<const Film>, boost::shared_ptr<cxml::Node>);
diff --git a/src/lib/film.cc b/src/lib/film.cc
index b78622a90..2bb8b3155 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -246,7 +246,7 @@ Film::make_dcp ()
 		throw MissingSettingError (_("container"));
 	}
 
-	if (_playlist->content().empty ()) {
+	if (content_without_loop().empty()) {
 		throw StringError (_("You must add some content to the DCP before creating it"));
 	}
 
@@ -729,9 +729,9 @@ Film::playlist () const
 }
 
 Playlist::ContentList
-Film::content () const
+Film::content_without_loop () const
 {
-	return _playlist->content ();
+	return _playlist->content_without_loop ();
 }
 
 void
@@ -769,9 +769,15 @@ Film::remove_content (shared_ptr<Content> c)
 }
 
 Time
-Film::length () const
+Film::length_with_loop () const
 {
-	return _playlist->length ();
+	return _playlist->length_with_loop ();
+}
+
+Time
+Film::length_without_loop () const
+{
+	return _playlist->length_without_loop ();
 }
 
 bool
diff --git a/src/lib/film.h b/src/lib/film.h
index bd9dcc88d..1f3899885 100644
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -106,9 +106,10 @@ public:
 
 	/* Proxies for some Playlist methods */
 
-	Playlist::ContentList content () const;
+	Playlist::ContentList content_without_loop () const;
 
-	Time length () const;
+	Time length_with_loop () const;
+	Time length_without_loop () const;
 	bool has_subtitles () const;
 	OutputVideoFrame best_dcp_video_frame_rate () const;
 
diff --git a/src/lib/imagemagick_content.cc b/src/lib/imagemagick_content.cc
index ae3e18137..79623b21c 100644
--- a/src/lib/imagemagick_content.cc
+++ b/src/lib/imagemagick_content.cc
@@ -76,7 +76,7 @@ ImageMagickContent::examine (shared_ptr<Job> job)
 	
 	shared_ptr<ImageMagickExaminer> examiner (new ImageMagickExaminer (film, shared_from_this()));
 
-	set_video_length (Config::instance()->default_still_length() * 24);
+	set_video_length (Config::instance()->default_still_length() * video_frame_rate());
 	take_from_video_examiner (examiner);
 }
 
@@ -97,8 +97,8 @@ ImageMagickContent::length () const
 	shared_ptr<const Film> film = _film.lock ();
 	assert (film);
 	
-	FrameRateConversion frc (24, film->dcp_video_frame_rate ());
-	return video_length() * frc.factor() * TIME_HZ / film->dcp_video_frame_rate ();
+	FrameRateConversion frc (video_frame_rate(), film->dcp_video_frame_rate ());
+	return video_length() * frc.factor() * TIME_HZ / video_frame_rate();
 }
 
 string
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 4ad930610..f5212f8d0 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -45,7 +45,7 @@ using boost::shared_ptr;
 using boost::weak_ptr;
 using boost::dynamic_pointer_cast;
 
-//#define DEBUG_PLAYER 1
+#define DEBUG_PLAYER 1
 
 class Piece
 {
@@ -401,7 +401,7 @@ Player::setup_pieces ()
 
 	_pieces.clear ();
 
-	Playlist::ContentList content = _playlist->content ();
+	Playlist::ContentList content = _playlist->content_with_loop ();
 	sort (content.begin(), content.end(), ContentSorter ());
 
 	for (Playlist::ContentList::iterator i = content.begin(); i != content.end(); ++i) {
diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc
index 8c4a7f7d7..172b6fbb9 100644
--- a/src/lib/playlist.cc
+++ b/src/lib/playlist.cc
@@ -28,6 +28,7 @@
 #include "ffmpeg_content.h"
 #include "imagemagick_decoder.h"
 #include "imagemagick_content.h"
+#include "content_factory.h"
 #include "job.h"
 #include "config.h"
 #include "util.h"
@@ -119,19 +120,7 @@ Playlist::set_from_xml (shared_ptr<const Film> film, shared_ptr<const cxml::Node
 {
 	list<shared_ptr<cxml::Node> > c = node->node_children ("Content");
 	for (list<shared_ptr<cxml::Node> >::iterator i = c.begin(); i != c.end(); ++i) {
-		string const type = (*i)->string_child ("Type");
-
-		boost::shared_ptr<Content> content;
-
-		if (type == "FFmpeg") {
-			content.reset (new FFmpegContent (film, *i));
-		} else if (type == "ImageMagick") {
-			content.reset (new ImageMagickContent (film, *i));
-		} else if (type == "Sndfile") {
-			content.reset (new SndfileContent (film, *i));
-		}
-
-		_content.push_back (content);
+		_content.push_back (content_factory (film, *i));
 	}
 
 	reconnect ();
@@ -257,7 +246,7 @@ Playlist::best_dcp_frame_rate () const
 }
 
 Time
-Playlist::length () const
+Playlist::length_without_loop () const
 {
 	Time len = 0;
 	for (ContentList::const_iterator i = _content.begin(); i != _content.end(); ++i) {
@@ -267,6 +256,12 @@ Playlist::length () const
 	return len;
 }
 
+Time
+Playlist::length_with_loop () const
+{
+	return length_without_loop() * _loop;
+}
+
 void
 Playlist::reconnect ()
 {
@@ -305,3 +300,30 @@ ContentSorter::operator() (shared_ptr<Content> a, shared_ptr<Content> b)
 {
 	return a->start() < b->start();
 }
+
+/** @return content in an undefined order, not taking looping into account */
+Playlist::ContentList
+Playlist::content_without_loop () const
+{
+	return _content;
+}
+
+/** @return content in an undefined order, taking looping into account */
+Playlist::ContentList
+Playlist::content_with_loop () const
+{
+	ContentList looped = _content;
+	Time const length = length_without_loop ();
+
+	Time offset = length;
+	for (int i = 1; i < _loop; ++i) {
+		for (ContentList::const_iterator i = _content.begin(); i != _content.end(); ++i) {
+			shared_ptr<Content> copy = (*i)->clone ();
+			copy->set_start (copy->start() + offset);
+			looped.push_back (copy);
+		}
+		offset += length;
+	}
+	
+	return looped;
+}
diff --git a/src/lib/playlist.h b/src/lib/playlist.h
index a31b6826e..330681c56 100644
--- a/src/lib/playlist.h
+++ b/src/lib/playlist.h
@@ -41,10 +41,7 @@ class Region;
  *  @brief A set of content files (video and audio), with knowledge of how they should be arranged into
  *  a DCP.
  *
- * This class holds Content objects, and it knows how they should be arranged.	At the moment
- * the ordering is implicit; video content is placed sequentially, and audio content is taken
- * from the video unless any sound-only files are present.  If sound-only files exist, they
- * are played simultaneously (i.e. they can be split up into multiple files for different channels)
+ * This class holds Content objects, and it knows how they should be arranged.
  */
 
 struct ContentSorter
@@ -67,10 +64,9 @@ public:
 	bool has_subtitles () const;
 
 	typedef std::vector<boost::shared_ptr<Content> > ContentList;
-	
-	ContentList content () const {
-		return _content;
-	}
+
+	ContentList content_without_loop () const;
+	ContentList content_with_loop () const;
 
 	std::string video_identifier () const;
 
@@ -80,7 +76,9 @@ public:
 	
 	void set_loop (int l);
 
-	Time length () const;
+	Time length_without_loop () const;
+	Time length_with_loop () const;
+	
 	int best_dcp_frame_rate () const;
 	Time video_end () const;
 
diff --git a/src/lib/transcode_job.cc b/src/lib/transcode_job.cc
index 6d5edd7c0..f0faf7c63 100644
--- a/src/lib/transcode_job.cc
+++ b/src/lib/transcode_job.cc
@@ -111,6 +111,6 @@ TranscodeJob::remaining_time () const
 	}
 
 	/* Compute approximate proposed length here, as it's only here that we need it */
-	OutputVideoFrame const left = _film->time_to_video_frames (_film->length ()) - _transcoder->video_frames_out();
+	OutputVideoFrame const left = _film->time_to_video_frames (_film->length_with_loop ()) - _transcoder->video_frames_out();
 	return left / fps;
 }
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 6601fb2fe..b3d2fdb1c 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -204,8 +204,10 @@ try
 			}
 			lock.lock ();
 			
-			if (_film->length ()) {
-				_job->set_progress (float(_full_written + _fake_written + _repeat_written) / _film->time_to_video_frames (_film->length()));
+			if (_film->length_with_loop()) {
+				_job->set_progress (
+					float (_full_written + _fake_written + _repeat_written) / _film->time_to_video_frames (_film->length_with_loop())
+					);
 			}
 
 			++_last_written_frame;
@@ -358,15 +360,19 @@ Writer::check_existing_picture_mxf ()
 		/* Read the data from the MXF and hash it */
 		fseek (mxf, info.offset, SEEK_SET);
 		EncodedData data (info.size);
-		fread (data.data(), 1, data.size(), mxf);
-		string const existing_hash = md5_digest (data.data(), data.size());
+		size_t const read = fread (data.data(), 1, data.size(), mxf);
+		if (read != static_cast<size_t> (data.size ())) {
+			_film->log()->log (String::compose ("Existing frame %1 is incomplete", _first_nonexistant_frame));
+			break;
+		}
 		
+		string const existing_hash = md5_digest (data.data(), data.size());
 		if (existing_hash != info.hash) {
-			_film->log()->log (String::compose (N_("Existing frame %1 failed hash check"), _first_nonexistant_frame));
+			_film->log()->log (String::compose ("Existing frame %1 failed hash check", _first_nonexistant_frame));
 			break;
 		}
 
-		_film->log()->log (String::compose (N_("Have existing frame %1"), _first_nonexistant_frame));
+		_film->log()->log (String::compose ("Have existing frame %1", _first_nonexistant_frame));
 		++_first_nonexistant_frame;
 	}
 
diff --git a/src/lib/wscript b/src/lib/wscript
index 0d3f2913c..2bedc1fce 100644
--- a/src/lib/wscript
+++ b/src/lib/wscript
@@ -10,6 +10,7 @@ sources = """
           audio_mapping.cc
           config.cc
           content.cc
+          content_factory.cc
           cross.cc
           dci_metadata.cc
           dcp_content_type.cc