Guess video range better when importing 2.14.x projects (#2227).

8 files changed, 58 insertions, 12 deletions

diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 6df0588e9..f8639cef9 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -95,7 +95,7 @@ DCPContent::DCPContent (boost::filesystem::path p)
 DCPContent::DCPContent (cxml::ConstNodePtr node, int version)
 	: Content (node)
 {
-	video = VideoContent::from_xml (this, node, version);
+	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 	audio = AudioContent::from_xml (this, node, version);
 	list<string> notes;
 	text = TextContent::from_xml (this, node, version, notes);
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 9017ad605..c791309ad 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -90,7 +90,11 @@ get_optional_enum (cxml::ConstNodePtr node, string name)
 FFmpegContent::FFmpegContent (cxml::ConstNodePtr node, int version, list<string>& notes)
 	: Content (node)
 {
-	video = VideoContent::from_xml (this, node, version);
+	_color_range = get_optional_enum<AVColorRange>(node, "ColorRange");
+
+	VideoRange const video_range_hint = (_color_range && *_color_range == AVCOL_RANGE_JPEG) ? VideoRange::FULL : VideoRange::VIDEO;
+
+	video = VideoContent::from_xml (this, node, version, video_range_hint);
 	audio = AudioContent::from_xml (this, node, version);
 	text = TextContent::from_xml (this, node, version, notes);
 
@@ -124,7 +128,6 @@ FFmpegContent::FFmpegContent (cxml::ConstNodePtr node, int version, list<string>
 		_first_video = ContentTime (f.get ());
 	}
 
-	_color_range = get_optional_enum<AVColorRange>(node, "ColorRange");
 	_color_primaries = get_optional_enum<AVColorPrimaries>(node, "ColorPrimaries");
 	_color_trc = get_optional_enum<AVColorTransferCharacteristic>(node, "ColorTransferCharacteristic");
 	_colorspace = get_optional_enum<AVColorSpace>(node, "Colorspace");
diff --git a/src/lib/image_content.cc b/src/lib/image_content.cc
index 9464babd4..d8f482a3b 100644
--- a/src/lib/image_content.cc
+++ b/src/lib/image_content.cc
@@ -61,7 +61,7 @@ ImageContent::ImageContent (boost::filesystem::path p)
 ImageContent::ImageContent (cxml::ConstNodePtr node, int version)
 	: Content (node)
 {
-	video = VideoContent::from_xml (this, node, version);
+	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 }
 
 
diff --git a/src/lib/video_content.cc b/src/lib/video_content.cc
index 51d6ba418..86efeee7b 100644
--- a/src/lib/video_content.cc
+++ b/src/lib/video_content.cc
@@ -86,17 +86,20 @@ VideoContent::VideoContent (Content* parent)
 }
 
 
+/** @param video_range_hint Video range to use if none is given in the XML */
 shared_ptr<VideoContent>
-VideoContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version)
+VideoContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, VideoRange video_range_hint)
 {
 	if (!node->optional_number_child<int> ("VideoWidth")) {
 		return {};
 	}
 
-	return make_shared<VideoContent>(parent, node, version);
+	return make_shared<VideoContent>(parent, node, version, video_range_hint);
 }
 
-VideoContent::VideoContent (Content* parent, cxml::ConstNodePtr node, int version)
+
+/** @param video_range_hint Video range to use if none is given in the XML */
+VideoContent::VideoContent (Content* parent, cxml::ConstNodePtr node, int version, VideoRange video_range_hint)
 	: ContentPart (parent)
 {
 	_size.width = node->number_child<int> ("VideoWidth");
@@ -185,8 +188,12 @@ VideoContent::VideoContent (Content* parent, cxml::ConstNodePtr node, int versio
 		_fade_in = _fade_out = 0;
 	}
 
-	_range = VideoRange::FULL;
-	if (node->optional_string_child("Range").get_value_or("full") == "video") {
+	auto video_range = node->optional_string_child("Range");
+	if (!video_range) {
+		_range = video_range_hint;
+	} else if (*video_range == "full") {
+		_range = VideoRange::FULL;
+	} else {
 		_range = VideoRange::VIDEO;
 	}
 
diff --git a/src/lib/video_content.h b/src/lib/video_content.h
index 2adf941d9..3ec884578 100644
--- a/src/lib/video_content.h
+++ b/src/lib/video_content.h
@@ -60,7 +60,7 @@ class VideoContent : public ContentPart, public std::enable_shared_from_this<Vid
 {
 public:
 	explicit VideoContent (Content* parent);
-	VideoContent (Content* parent, cxml::ConstNodePtr, int);
+	VideoContent (Content* parent, cxml::ConstNodePtr node, int version, VideoRange video_range_hint);
 	VideoContent (Content* parent, std::vector<std::shared_ptr<Content>>);
 
 	void as_xml (xmlpp::Node *) const;
@@ -216,7 +216,7 @@ public:
 	void modify_position (std::shared_ptr<const Film> film, dcpomatic::DCPTime& pos) const;
 	void modify_trim_start (dcpomatic::ContentTime& pos) const;
 
-	static std::shared_ptr<VideoContent> from_xml (Content* parent, cxml::ConstNodePtr, int);
+	static std::shared_ptr<VideoContent> from_xml (Content* parent, cxml::ConstNodePtr node, int version, VideoRange video_range_hint);
 
 private:
 
diff --git a/src/lib/video_mxf_content.cc b/src/lib/video_mxf_content.cc
index c7ee69d89..c6165172a 100644
--- a/src/lib/video_mxf_content.cc
+++ b/src/lib/video_mxf_content.cc
@@ -51,7 +51,7 @@ VideoMXFContent::VideoMXFContent (boost::filesystem::path path)
 VideoMXFContent::VideoMXFContent (cxml::ConstNodePtr node, int version)
 	: Content (node)
 {
-	video = VideoContent::from_xml (this, node, version);
+	video = VideoContent::from_xml (this, node, version, VideoRange::FULL);
 }
 
 
diff --git a/test/data b/test/data
-Subproject 796f3e657f63985b360447a437c1d30be1ccaa2
+Subproject 6b3ced6ff12cca13b1765c16bbdface28040e44
diff --git a/test/film_metadata_test.cc b/test/film_metadata_test.cc
index 62cb5cf1d..5c44c2a91 100644
--- a/test/film_metadata_test.cc
+++ b/test/film_metadata_test.cc
@@ -32,6 +32,7 @@
 #include "lib/film.h"
 #include "lib/ratio.h"
 #include "lib/text_content.h"
+#include "lib/video_content.h"
 #include "test.h"
 #include <boost/date_time.hpp>
 #include <boost/filesystem.hpp>
@@ -174,3 +175,38 @@ BOOST_AUTO_TEST_CASE (metadata_loads_from_2_14_x_4)
 	BOOST_REQUIRE (!film->luminance());
 }
 
+
+BOOST_AUTO_TEST_CASE (metadata_video_range_guessed_for_dcp)
+{
+	namespace fs = boost::filesystem;
+	auto film = make_shared<Film>(fs::path("test/data/214x_dcp"));
+	film->read_metadata();
+
+	BOOST_REQUIRE_EQUAL(film->content().size(), 1U);
+	BOOST_REQUIRE(film->content()[0]->video);
+	BOOST_CHECK(film->content()[0]->video->range() == VideoRange::FULL);
+}
+
+
+BOOST_AUTO_TEST_CASE (metadata_video_range_guessed_for_mp4_with_unknown_range)
+{
+	namespace fs = boost::filesystem;
+	auto film = make_shared<Film>(fs::path("test/data/214x_mp4"));
+	film->read_metadata();
+
+	BOOST_REQUIRE_EQUAL(film->content().size(), 1U);
+	BOOST_REQUIRE(film->content()[0]->video);
+	BOOST_CHECK(film->content()[0]->video->range() == VideoRange::VIDEO);
+}
+
+
+BOOST_AUTO_TEST_CASE (metadata_video_range_guessed_for_png)
+{
+	namespace fs = boost::filesystem;
+	auto film = make_shared<Film>(fs::path("test/data/214x_png"));
+	film->read_metadata();
+
+	BOOST_REQUIRE_EQUAL(film->content().size(), 1U);
+	BOOST_REQUIRE(film->content()[0]->video);
+	BOOST_CHECK(film->content()[0]->video->range() == VideoRange::FULL);
+}