Rearrange checking (and re-examining) content.

Most importantly, checking of content for changes before making
a DCP is now done in the TranscodeJob (rather than being in a
separate job).  This makes things a little neater and also makes
the batch converter less confusing when you add a job whose content
has changed.

6 files changed, 69 insertions, 70 deletions

diff --git a/src/lib/check_content_change_job.cc b/src/lib/check_content_change_job.cc
index 967291bb3..500e03c1d 100644
--- a/src/lib/check_content_change_job.cc
+++ b/src/lib/check_content_change_job.cc
@@ -32,11 +32,8 @@ using std::list;
 using std::cout;
 using std::shared_ptr;
 
-/** @param gui true if we are running this job from the GUI, false if it's the CLI */
-CheckContentChangeJob::CheckContentChangeJob (shared_ptr<const Film> film, shared_ptr<Job> following, bool gui)
+CheckContentChangeJob::CheckContentChangeJob (shared_ptr<const Film> film)
 	: Job (film)
-	, _following (following)
-	, _gui (gui)
 {
 
 }
@@ -68,35 +65,12 @@ CheckContentChangeJob::run ()
 	std::copy_if (content.begin(), content.end(), std::back_inserter(changed), [](shared_ptr<Content> c) { return c->changed(); });
 
 	if (!changed.empty()) {
-		if (_gui) {
-			for (auto i: changed) {
-				JobManager::instance()->add(shared_ptr<Job>(new ExamineContentJob(_film, i)));
-			}
-			string m = _("Some files have been changed since they were added to the project.\n\nThese files will now be re-examined, so you may need to check their settings.");
-			if (_following) {
-				/* I'm assuming that _following is a make DCP job */
-				m += "  ";
-				m += _("Choose 'Make DCP' again when you have done this.");
-			}
-			set_message (m);
-		} else {
-			set_progress (1);
-			set_state (FINISHED_ERROR);
-			set_error (
-				_("Some files have been changed since they were added to the project.  Open the project in DCP-o-matic, check the settings, then save it before trying again."),
-				""
-				);
-			return;
+		for (auto i: changed) {
+			JobManager::instance()->add(make_shared<ExamineContentJob>(_film, i));
 		}
-	} else if (_following) {
-		JobManager::instance()->add (_following);
+		set_message (_("Some files have been changed since they were added to the project.\n\nThese files will now be re-examined, so you may need to check their settings."));
 	}
 
-	/* Only set this job as finished once we have added the following job, otherwise I think
-	   it's possible that the tests will sporadically fail if they check for all jobs being
-	   complete in the gap between this one finishing and _following being added.
-	*/
-
 	set_progress (1);
 	set_state (FINISHED_OK);
 }
diff --git a/src/lib/check_content_change_job.h b/src/lib/check_content_change_job.h
index 739d2651f..b3cdf594c 100644
--- a/src/lib/check_content_change_job.h
+++ b/src/lib/check_content_change_job.h
@@ -27,14 +27,10 @@
 class CheckContentChangeJob : public Job
 {
 public:
-	CheckContentChangeJob (std::shared_ptr<const Film>, std::shared_ptr<Job> following = std::shared_ptr<Job>(), bool gui = true);
+	CheckContentChangeJob (std::shared_ptr<const Film>);
 	~CheckContentChangeJob ();
 
 	std::string name () const;
 	std::string json_name () const;
 	void run ();
-
-private:
-	std::shared_ptr<Job> _following;
-	bool _gui;
 };
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 64fc04f5d..d0f0106c4 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -377,12 +377,9 @@ Film::subtitle_analysis_path (shared_ptr<const Content> content) const
 }
 
 
-/** Add suitable Jobs to the JobManager to create a DCP for this Film.
- *  @param gui true if this is being called from a GUI tool.
- *  @param check true to check the content in the project for changes before making the DCP.
- */
+/** Add suitable Jobs to the JobManager to create a DCP for this Film */
 void
-Film::make_dcp (bool gui, bool check)
+Film::make_dcp (TranscodeJob::ChangedBehaviour behaviour)
 {
 	if (dcp_name().find ("/") != string::npos) {
 		throw BadSettingError (_("name"), _("Cannot contain slashes"));
@@ -438,14 +435,9 @@ Film::make_dcp (bool gui, bool check)
 	}
 	LOG_GENERAL ("J2K bandwidth %1", j2k_bandwidth());
 
-	auto tj = make_shared<TranscodeJob>(shared_from_this());
+	auto tj = make_shared<TranscodeJob>(shared_from_this(), behaviour);
 	tj->set_encoder (make_shared<DCPEncoder>(shared_from_this(), tj));
-	if (check) {
-		auto cc = make_shared<CheckContentChangeJob>(shared_from_this(), tj, gui);
-		JobManager::instance()->add (cc);
-	} else {
-		JobManager::instance()->add (tj);
-	}
+	JobManager::instance()->add (tj);
 }
 
 /** Start a job to send our DCP to the configured TMS */
diff --git a/src/lib/film.h b/src/lib/film.h
index 5255e8355..78a66e17f 100644
--- a/src/lib/film.h
+++ b/src/lib/film.h
@@ -30,6 +30,7 @@
 #include "dcp_text_track.h"
 #include "frame_rate_change.h"
 #include "signaller.h"
+#include "transcode_job.h"
 #include "types.h"
 #include "util.h"
 #include <dcp/encrypted_kdm.h>
@@ -111,7 +112,7 @@ public:
 	boost::filesystem::path subtitle_analysis_path (std::shared_ptr<const Content>) const;
 
 	void send_dcp_to_tms ();
-	void make_dcp (bool gui = false, bool check = true);
+	void make_dcp (TranscodeJob::ChangedBehaviour behaviour);
 
 	/** @return Logger.
 	 *  It is safe to call this from any thread.
diff --git a/src/lib/transcode_job.cc b/src/lib/transcode_job.cc
index 8186b9e7c..68cd30e54 100644
--- a/src/lib/transcode_job.cc
+++ b/src/lib/transcode_job.cc
@@ -26,10 +26,12 @@
 
 #include "analytics.h"
 #include "compose.hpp"
+#include "content.h"
 #include "config.h"
 #include "dcp_encoder.h"
 #include "dcpomatic_log.h"
 #include "encoder.h"
+#include "examine_content_job.h"
 #include "film.h"
 #include "job_manager.h"
 #include "log.h"
@@ -52,8 +54,9 @@ using std::dynamic_pointer_cast;
 
 
 /** @param film Film to use */
-TranscodeJob::TranscodeJob (shared_ptr<const Film> film)
+TranscodeJob::TranscodeJob (shared_ptr<const Film> film, ChangedBehaviour changed)
 	: Job (film)
+	, _changed (changed)
 {
 
 }
@@ -90,6 +93,32 @@ void
 TranscodeJob::run ()
 {
 	try {
+		auto content = _film->content();
+		std::vector<shared_ptr<Content>> changed;
+		std::copy_if (content.begin(), content.end(), std::back_inserter(changed), [](shared_ptr<Content> c) { return c->changed(); });
+
+		if (!changed.empty()) {
+			switch (_changed) {
+			case ChangedBehaviour::EXAMINE_THEN_STOP:
+				for (auto i: changed) {
+					JobManager::instance()->add(make_shared<ExamineContentJob>(_film, i));
+				}
+				set_progress (1);
+				set_message (_("Some files have been changed since they were added to the project.\n\nThese files will now be re-examined, so you may need to check their settings before trying again."));
+				set_error (_("Files have changed since they were added to the project."), _("Check their new settings, then try again."));
+				set_state (FINISHED_ERROR);
+				return;
+			case ChangedBehaviour::STOP:
+				set_progress (1);
+				set_error (_("Files have changed since they were added to the project."), _("Open the project in DCP-o-matic, check the settings, then save it before trying again."));
+				set_state (FINISHED_ERROR);
+				return;
+			default:
+				LOG_GENERAL_NC (_("Some files have been changed since they were added to the project."));
+				break;
+			}
+		}
+
 		struct timeval start;
 		gettimeofday (&start, 0);
 		LOG_GENERAL_NC (N_("Transcode job starting"));
@@ -139,29 +168,17 @@ TranscodeJob::status () const
 		return Job::status ();
 	}
 
-
-	char buffer[256];
 	if (finished() || _encoder->finishing()) {
-		strncpy (buffer, Job::status().c_str(), 255);
-		buffer[255] = '\0';
-	} else {
-		snprintf (
-			buffer, sizeof(buffer), "%s; %" PRId64 "/%" PRId64 " frames",
-			Job::status().c_str(),
-			_encoder->frames_done(),
-			_film->length().frames_round (_film->video_frame_rate ())
-			);
-
-		optional<float> const fps = _encoder->current_rate ();
-		if (fps) {
-			char fps_buffer[64];
-			/// TRANSLATORS: fps here is an abbreviation for frames per second
-			snprintf (fps_buffer, sizeof(fps_buffer), _("; %.1f fps"), *fps);
-			strncat (buffer, fps_buffer, strlen(buffer) - 1);
-		}
+		return Job::status();
+	}
+
+	auto status = String::compose(_("%1; %2/%3 frames"), Job::status(), _encoder->frames_done(), _film->length().frames_round(_film->video_frame_rate()));
+	if (auto const fps = _encoder->current_rate()) {
+		/// TRANSLATORS: fps here is an abbreviation for frames per second
+		status += String::compose(_("; %1 fps"), dcp::locale_convert<string>(*fps, 1));
 	}
 
-	return buffer;
+	return status;
 }
 
 
diff --git a/src/lib/transcode_job.h b/src/lib/transcode_job.h
index 030e22bef..368a9b685 100644
--- a/src/lib/transcode_job.h
+++ b/src/lib/transcode_job.h
@@ -19,6 +19,10 @@
 */
 
 
+#ifndef DCPOMATIC_TRANSCODE_JOB_H
+#define DCPOMATIC_TRANSCODE_JOB_H
+
+
 /** @file src/transcode_job.h
  *  @brief A job which transcodes from one format to another.
  */
@@ -27,6 +31,10 @@
 #include "job.h"
 
 
+/* Defined by Windows */
+#undef IGNORE
+
+
 class Encoder;
 
 
@@ -36,7 +44,13 @@ class Encoder;
 class TranscodeJob : public Job
 {
 public:
-	explicit TranscodeJob (std::shared_ptr<const Film> film);
+	enum class ChangedBehaviour {
+		EXAMINE_THEN_STOP,
+		STOP,
+		IGNORE
+	};
+
+	explicit TranscodeJob (std::shared_ptr<const Film> film, ChangedBehaviour changed);
 	~TranscodeJob ();
 
 	std::string name () const override;
@@ -50,4 +64,9 @@ private:
 	int remaining_time () const override;
 
 	std::shared_ptr<Encoder> _encoder;
+	ChangedBehaviour _changed;
 };
+
+
+#endif
+