Simplify the FontIDAllocator a lot (#2827).

This is at the expense of forward compatibility, and the need to
re-examine subtitle content (losing custom fonts as we do so).

But it does mean that the code is simpler, and there's not this weird
growth of IDs where a DCP gets imported with some font, and then the ID
becomes 0_font, and if you do it again it's 0_0_font, and so on.

4 files changed, 14 insertions, 27 deletions

diff --git a/src/lib/check_content_job.cc b/src/lib/check_content_job.cc
index f37890abf..2028b01ac 100644
--- a/src/lib/check_content_job.cc
+++ b/src/lib/check_content_job.cc
@@ -70,7 +70,7 @@ CheckContentJob::run ()
 	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 (_film->last_written_by_earlier_than(2, 16, 15)) {
+	if (_film->last_written_by_earlier_than(2, 17, 17)) {
 		for (auto c: content) {
 			if (auto stf = dynamic_pointer_cast<StringTextFileContent>(c)) {
 				stf->check_font_ids();
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 0aeb1d041..b4e979481 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -851,9 +851,10 @@ DCPContent::check_font_ids()
 		return;
 	}
 
-	/* This might be called on a TextContent that already has the correct fonts
-	 * (e.g. if run from a build with a LastWrittenBy containing only a git hash)
-	 * so we'll get an error if we don't clear them out first.
+	/* This might be called on a TextContent that already has some fonts
+	 * (e.g. if run from a build with a LastWrittenBy containing only a git
+	 * hash, or from a version between 2.16.15 and 2.17.17) so we'll get an
+	 * error if we don't clear them out first.
 	 */
 	text[0]->clear_fonts();
 	DCPExaminer examiner(shared_from_this(), true);
diff --git a/src/lib/font_id_allocator.cc b/src/lib/font_id_allocator.cc
index 112dd262b..76b52e730 100644
--- a/src/lib/font_id_allocator.cc
+++ b/src/lib/font_id_allocator.cc
@@ -76,37 +76,23 @@ FontIDAllocator::add_font(int reel_index, string asset_id, string font_id)
 	if (!_default_font) {
 		_default_font = font;
 	}
-	_map[font] = 0;
+	_map[font] = {};
 }
 
 
 void
 FontIDAllocator::allocate()
 {
-	/* We'll first try adding <reel>_ to the start of the font ID, but if a reel has multiple
-	 * identical font IDs we will need to use some number that is not a reel ID.  Find the
-	 * first such number (1 higher than the highest reel index)
-	 */
-	auto next_unused = std::max_element(
-		_map.begin(),
-		_map.end(),
-		[] (std::pair<Font, int> const& a, std::pair<Font, int> const& b) {
-			return a.first.reel_index < b.first.reel_index;
-		})->first.reel_index + 1;
-
 	std::set<string> used_ids;
 
 	for (auto& font: _map) {
-		auto const proposed = String::compose("%1_%2", font.first.reel_index, font.first.font_id);
-		if (used_ids.find(proposed) != used_ids.end()) {
-			/* This ID was already used; we need to disambiguate it.  Do so by using
-			 * one of our unused prefixes.
-			 */
-			font.second = next_unused++;
-		} else {
-			/* This ID was not yet used */
-			font.second = font.first.reel_index;
+		auto proposed = font.first.font_id;
+		int prefix = 0;
+		while (used_ids.find(proposed) != used_ids.end()) {
+			proposed = String::compose("%1_%2", prefix++, font.first.font_id);
+			DCPOMATIC_ASSERT(prefix < 128);
 		}
+		font.second = proposed;
 		used_ids.insert(proposed);
 	}
 }
@@ -117,7 +103,7 @@ FontIDAllocator::font_id(int reel_index, string asset_id, string font_id) const
 {
 	auto iter = _map.find(Font(reel_index, asset_id, font_id));
 	DCPOMATIC_ASSERT(iter != _map.end());
-	return String::compose("%1_%2", iter->second, font_id);
+	return iter->second;
 }
 
 
diff --git a/src/lib/font_id_allocator.h b/src/lib/font_id_allocator.h
index fe4b9ef07..6737907c1 100644
--- a/src/lib/font_id_allocator.h
+++ b/src/lib/font_id_allocator.h
@@ -101,7 +101,7 @@ private:
 		std::string font_id;
 	};
 
-	std::map<Font, int> _map;
+	std::map<Font, std::string> _map;
 	boost::optional<Font> _default_font;
 };