Make DCPExaminer::size() optional and deal with the consequences.v2.16.78

This means we can fix the case of a VF having no known size in a nice way,
in turn fixing problems caused by the fix to #2775.

19 files changed, 110 insertions, 73 deletions

diff --git a/src/lib/copy_dcp_details_to_film.cc b/src/lib/copy_dcp_details_to_film.cc
index 669fc8ac9..f6ca08638 100644
--- a/src/lib/copy_dcp_details_to_film.cc
+++ b/src/lib/copy_dcp_details_to_film.cc
@@ -51,7 +51,9 @@ copy_dcp_settings_to_film(shared_ptr<const DCPContent> dcp, shared_ptr<Film> fil
 	film->set_three_d (dcp->three_d());
 
 	if (dcp->video) {
-		film->set_container (Ratio::nearest_from_ratio(dcp->video->size().ratio()));
+		if (auto size = dcp->video->size()) {
+			film->set_container(Ratio::nearest_from_ratio(size->ratio()));
+		}
 		film->set_resolution (dcp->resolution());
 		DCPOMATIC_ASSERT (dcp->video_frame_rate());
 		film->set_video_frame_rate (*dcp->video_frame_rate());
diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index e22f69f56..6185b3a19 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -830,7 +830,7 @@ DCPContent::kdm_timing_window_valid () const
 Resolution
 DCPContent::resolution () const
 {
-	if (video->size().width > 2048 || video->size().height > 1080) {
+	if (video->size() && (video->size()->width > 2048 || video->size()->height > 1080)) {
 		return Resolution::FOUR_K;
 	}
 
diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h
index 444fb7567..04fa31ea4 100644
--- a/src/lib/dcp_examiner.h
+++ b/src/lib/dcp_examiner.h
@@ -48,10 +48,8 @@ public:
 		return _video_frame_rate;
 	}
 
-	dcp::Size video_size () const override {
-		DCPOMATIC_ASSERT (_has_video);
-		DCPOMATIC_ASSERT (_video_size);
-		return *_video_size;
+	boost::optional<dcp::Size> video_size() const override {
+		return _video_size;
 	}
 
 	Frame video_length () const override {
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 20fe7776b..c788e7cb1 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -521,7 +521,7 @@ FFmpegContent::set_default_colour_conversion ()
 		video->set_colour_conversion (PresetColourConversion::from_id ("rec2020").conversion);
 		break;
 	default:
-		if (s.width < 1080) {
+		if (s && s->width < 1080) {
 			video->set_colour_conversion (PresetColourConversion::from_id ("rec601").conversion);
 		} else {
 			video->set_colour_conversion (PresetColourConversion::from_id ("rec709").conversion);
diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 0d43156c0..7f7a07863 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -828,11 +828,14 @@ FFmpegDecoder::process_ass_subtitle (string ass, ContentTime from)
 	}
 
 	sub::RawSubtitle base;
+	auto video_size = _ffmpeg_content->video->size();
+	DCPOMATIC_ASSERT(video_size);
+
 	auto raw = sub::SSAReader::parse_line (
 		base,
 		text,
-		_ffmpeg_content->video->size().width,
-		_ffmpeg_content->video->size().height,
+		video_size->width,
+		video_size->height,
 		sub::Colour(1, 1, 1)
 		);
 
diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc
index ed6f21094..15cb14ad5 100644
--- a/src/lib/ffmpeg_examiner.cc
+++ b/src/lib/ffmpeg_examiner.cc
@@ -305,7 +305,7 @@ FFmpegExaminer::video_frame_rate () const
 }
 
 
-dcp::Size
+optional<dcp::Size>
 FFmpegExaminer::video_size () const
 {
 	return dcp::Size (video_codec_context()->width, video_codec_context()->height);
diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h
index 4747929b5..45313ec18 100644
--- a/src/lib/ffmpeg_examiner.h
+++ b/src/lib/ffmpeg_examiner.h
@@ -38,7 +38,7 @@ public:
 	bool has_video () const override;
 
 	boost::optional<double> video_frame_rate () const override;
-	dcp::Size video_size () const override;
+	boost::optional<dcp::Size> video_size() const override;
 	Frame video_length () const override;
 	boost::optional<double> sample_aspect_ratio () const override;
 	bool yuv () const override;
diff --git a/src/lib/film.cc b/src/lib/film.cc
index 0bba13e28..d9ab6e2a3 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -943,10 +943,12 @@ Film::isdcf_name (bool if_created_now) const
 	if (dcp_content_type() && dcp_content_type()->libdcp_kind() != dcp::ContentKind::TRAILER) {
 		auto first_video = std::find_if(content_list.begin(), content_list.end(), [](shared_ptr<Content> c) { return static_cast<bool>(c->video); });
 		if (first_video != content_list.end()) {
-			auto first_ratio = lrintf((*first_video)->video->scaled_size(frame_size()).ratio() * 100);
-			auto container_ratio = lrintf(container()->ratio() * 100);
-			if (first_ratio != container_ratio) {
-				isdcf_name += "-" + dcp::raw_convert<string>(first_ratio);
+			if (auto scaled_size = (*first_video)->video->scaled_size(frame_size())) {
+				auto first_ratio = lrintf(scaled_size->ratio() * 100);
+				auto container_ratio = lrintf(container()->ratio() * 100);
+				if (first_ratio != container_ratio) {
+					isdcf_name += "-" + dcp::raw_convert<string>(first_ratio);
+				}
 			}
 		}
 	}
@@ -1435,13 +1437,13 @@ Film::maybe_set_container_and_resolution ()
 		}
 	}
 
-	if (video) {
+	if (video && video->size()) {
 		/* This is the only piece of video content in this Film.  Use it to make a guess for
 		 * DCP container size and resolution, unless the user has already explicitly set these
 		 * things.
 		 */
 		if (!_user_explicit_container) {
-			if (video->size().ratio() > 2.3) {
+			if (video->size()->ratio() > 2.3) {
 				set_container (Ratio::from_id("239"), false);
 			} else {
 				set_container (Ratio::from_id("185"), false);
@@ -1449,7 +1451,7 @@ Film::maybe_set_container_and_resolution ()
 		}
 
 		if (!_user_explicit_resolution) {
-			if (video->size_after_crop().width > 2048 || video->size_after_crop().height > 1080) {
+			if (video->size_after_crop()->width > 2048 || video->size_after_crop()->height > 1080) {
 				set_resolution (Resolution::FOUR_K, false);
 			} else {
 				set_resolution (Resolution::TWO_K, false);
@@ -1647,9 +1649,10 @@ Film::active_area () const
 
 	for (auto i: content()) {
 		if (i->video) {
-			dcp::Size s = i->video->scaled_size (frame);
-			active.width = max(active.width, s.width);
-			active.height = max(active.height, s.height);
+			if (auto s = i->video->scaled_size(frame)) {
+				active.width = max(active.width, s->width);
+				active.height = max(active.height, s->height);
+			}
 		}
 	}
 
diff --git a/src/lib/hints.cc b/src/lib/hints.cc
index 7537317d4..bbd5ae5d5 100644
--- a/src/lib/hints.cc
+++ b/src/lib/hints.cc
@@ -128,8 +128,8 @@ Hints::check_incorrect_container ()
 	int narrower_than_scope = 0;
 	int scope = 0;
 	for (auto i: film()->content()) {
-		if (i->video) {
-			auto const r = Ratio::nearest_from_ratio(i->video->scaled_size(film()->frame_size()).ratio());
+		if (i->video && i->video->size()) {
+			auto const r = Ratio::nearest_from_ratio(i->video->scaled_size(film()->frame_size())->ratio());
 			if (r && r->id() == "239") {
 				++scope;
 			} else if (r && r->id() != "239" && r->id() != "235" && r->id() != "190") {
diff --git a/src/lib/image_decoder.cc b/src/lib/image_decoder.cc
index 26a9ad624..ce5c8757f 100644
--- a/src/lib/image_decoder.cc
+++ b/src/lib/image_decoder.cc
@@ -73,7 +73,9 @@ ImageDecoder::pass ()
 			/* We can't extract image size from a JPEG2000 codestream without decoding it,
 			   so pass in the image content's size here.
 			*/
-			_image = make_shared<J2KImageProxy>(path, _image_content->video->size(), pf);
+			auto size = _image_content->video->size();
+			DCPOMATIC_ASSERT(size);
+			_image = make_shared<J2KImageProxy>(path, *size, pf);
 		} else {
 			_image = make_shared<FFmpegImageProxy>(path);
 		}
diff --git a/src/lib/image_examiner.cc b/src/lib/image_examiner.cc
index ac2c95a08..15a0b043d 100644
--- a/src/lib/image_examiner.cc
+++ b/src/lib/image_examiner.cc
@@ -77,10 +77,10 @@ ImageExaminer::ImageExaminer (shared_ptr<const Film> film, shared_ptr<const Imag
 }
 
 
-dcp::Size
+optional<dcp::Size>
 ImageExaminer::video_size () const
 {
-	return _video_size.get ();
+	return _video_size;
 }
 
 
diff --git a/src/lib/image_examiner.h b/src/lib/image_examiner.h
index 53fab327e..54fca7ed1 100644
--- a/src/lib/image_examiner.h
+++ b/src/lib/image_examiner.h
@@ -31,7 +31,7 @@ public:
 		return true;
 	}
 	boost::optional<double> video_frame_rate () const override;
-	dcp::Size video_size () const override;
+	boost::optional<dcp::Size> video_size() const override;
 	Frame video_length () const override {
 		return _video_length;
 	}
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 0796fbceb..c03cb97a5 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -1094,13 +1094,16 @@ Player::video (weak_ptr<Piece> weak_piece, ContentVideo video)
 
 	auto const content_video = piece->content->video;
 
+	auto scaled_size = content_video->scaled_size(film->frame_size());
+	DCPOMATIC_ASSERT(scaled_size);
+
 	for (auto eyes: eyes_to_emit) {
 		_last_video[weak_piece] = std::make_shared<PlayerVideo>(
 			video.image,
 			content_video->actual_crop(),
 			content_video->fade(film, video.frame),
 			scale_for_display(
-				content_video->scaled_size(film->frame_size()),
+				*scaled_size,
 				_video_container_size,
 				film->frame_size(),
 				content_video->pixel_quanta()
diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index d45bf9f43..35c5d3daa 100644
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -362,8 +362,13 @@ PlayerVideo::reset_metadata (shared_ptr<const Film> film, dcp::Size player_video
 
 	_crop = content->video->actual_crop();
 	_fade = content->video->fade(film, _video_frame.get());
+	auto const size = content->video->scaled_size(film->frame_size());
+	if (!size) {
+		return false;
+	}
+
 	_inter_size = scale_for_display(
-		content->video->scaled_size(film->frame_size()),
+		*size,
 		player_video_container_size,
 		film->frame_size(),
 		content->video->pixel_quanta()
diff --git a/src/lib/video_content.cc b/src/lib/video_content.cc
index 6c3f23a57..91ed11855 100644
--- a/src/lib/video_content.cc
+++ b/src/lib/video_content.cc
@@ -102,8 +102,7 @@ VideoContent::from_xml (Content* parent, cxml::ConstNodePtr node, int version, V
 VideoContent::VideoContent (Content* parent, cxml::ConstNodePtr node, int version, VideoRange video_range_hint)
 	: ContentPart (parent)
 {
-	_size.width = node->number_child<int> ("VideoWidth");
-	_size.height = node->number_child<int> ("VideoHeight");
+	_size = dcp::Size(node->number_child<int>("VideoWidth"), node->number_child<int>("VideoHeight"));
 
 	_use = node->optional_bool_child("Use").get_value_or(true);
 	_length = node->number_child<Frame> ("VideoLength");
@@ -154,10 +153,12 @@ VideoContent::VideoContent (Content* parent, cxml::ConstNodePtr node, int versio
 		if (scale) {
 			if (*scale) {
 				/* This is what we used to call "no stretch" */
-				_legacy_ratio = _size.ratio();
+				DCPOMATIC_ASSERT(_size);
+				_legacy_ratio = _size->ratio();
 			} else {
 				/* This is what we used to call "no scale" */
-				_custom_size = _size;
+				DCPOMATIC_ASSERT(_size);
+				_custom_size = *_size;
 			}
 		}
 
@@ -277,8 +278,10 @@ VideoContent::as_xml (xmlpp::Node* node) const
 	boost::mutex::scoped_lock lm (_mutex);
 	node->add_child("Use")->add_child_text (_use ? "1" : "0");
 	node->add_child("VideoLength")->add_child_text (raw_convert<string> (_length));
-	node->add_child("VideoWidth")->add_child_text (raw_convert<string> (_size.width));
-	node->add_child("VideoHeight")->add_child_text (raw_convert<string> (_size.height));
+	if (_size) {
+		node->add_child("VideoWidth")->add_child_text(raw_convert<string>(_size->width));
+		node->add_child("VideoHeight")->add_child_text(raw_convert<string>(_size->height));
+	}
 	node->add_child("VideoFrameType")->add_child_text (video_frame_type_to_string (_frame_type));
 	if (_sample_aspect_ratio) {
 		node->add_child("SampleAspectRatio")->add_child_text (raw_convert<string> (_sample_aspect_ratio.get ()));
@@ -369,11 +372,12 @@ VideoContent::identifier () const
 string
 VideoContent::technical_summary () const
 {
+	string const size_string = size() ? String::compose("%1x%2", size()->width, size()->height) : _("unknown");
+
 	string s = String::compose (
-		N_("video: length %1 frames, size %2x%3"),
+		N_("video: length %1 frames, size %2"),
 		length_after_3d_combine(),
-		size().width,
-		size().height
+		size_string
 		);
 
 	if (sample_aspect_ratio ()) {
@@ -383,31 +387,40 @@ VideoContent::technical_summary () const
 	return s;
 }
 
-dcp::Size
+optional<dcp::Size>
 VideoContent::size_after_3d_split () const
 {
 	auto const s = size ();
+	if (!s) {
+		return {};
+	}
+
 	switch (frame_type ()) {
 	case VideoFrameType::TWO_D:
 	case VideoFrameType::THREE_D:
 	case VideoFrameType::THREE_D_ALTERNATE:
 	case VideoFrameType::THREE_D_LEFT:
 	case VideoFrameType::THREE_D_RIGHT:
-		return s;
+		return *s;
 	case VideoFrameType::THREE_D_LEFT_RIGHT:
-		return dcp::Size (s.width / 2, s.height);
+		return dcp::Size(s->width / 2, s->height);
 	case VideoFrameType::THREE_D_TOP_BOTTOM:
-		return dcp::Size (s.width, s.height / 2);
+		return dcp::Size(s->width, s->height / 2);
 	}
 
 	DCPOMATIC_ASSERT (false);
 }
 
 /** @return Video size after 3D split and crop */
-dcp::Size
+optional<dcp::Size>
 VideoContent::size_after_crop () const
 {
-	return actual_crop().apply(size_after_3d_split());
+	auto const after_3d = size_after_3d_split();
+	if (!after_3d) {
+		return {};
+	}
+
+	return actual_crop().apply(*after_3d);
 }
 
 
@@ -440,15 +453,15 @@ VideoContent::processing_description (shared_ptr<const Film> film)
 	string d;
 	char buffer[256];
 
-	if (size().width && size().height) {
+	if (size() && size()->width && size()->height) {
 		d += String::compose (
 			_("Content video is %1x%2"),
-			size_after_3d_split().width,
-			size_after_3d_split().height
+			size_after_3d_split()->width,
+			size_after_3d_split()->height
 			);
 
 
-		double ratio = size_after_3d_split().ratio ();
+		auto ratio = size_after_3d_split()->ratio();
 
 		if (sample_aspect_ratio ()) {
 			snprintf (buffer, sizeof(buffer), _(", pixel aspect ratio %.2f:1"), sample_aspect_ratio().get());
@@ -464,29 +477,31 @@ VideoContent::processing_description (shared_ptr<const Film> film)
 
 	if ((crop.left || crop.right || crop.top || crop.bottom) && size() != dcp::Size(0, 0)) {
 		auto const cropped = size_after_crop ();
-		d += String::compose (
-			_("\nCropped to %1x%2"),
-			cropped.width, cropped.height
-			);
+		if (cropped) {
+			d += String::compose (
+				_("\nCropped to %1x%2"),
+				cropped->width, cropped->height
+				);
 
-		snprintf (buffer, sizeof(buffer), " (%.2f:1)", cropped.ratio());
-		d += buffer;
+			snprintf(buffer, sizeof(buffer), " (%.2f:1)", cropped->ratio());
+			d += buffer;
+		}
 	}
 
 	auto const container_size = film->frame_size ();
 	auto const scaled = scaled_size (container_size);
 
-	if (scaled != size_after_crop ()) {
+	if (scaled && *scaled != size_after_crop()) {
 		d += String::compose (
 			_("\nScaled to %1x%2"),
-			scaled.width, scaled.height
+			scaled->width, scaled->height
 			);
 
-		snprintf (buffer, sizeof(buffer), _(" (%.2f:1)"), scaled.ratio());
+		snprintf (buffer, sizeof(buffer), _(" (%.2f:1)"), scaled->ratio());
 		d += buffer;
 	}
 
-	if (scaled != container_size) {
+	if (scaled && *scaled != container_size) {
 		d += String::compose (
 			_("\nPadded with black to fit container %1 (%2x%3)"),
 			film->container()->container_nickname (),
@@ -514,7 +529,9 @@ void
 VideoContent::add_properties (list<UserProperty>& p) const
 {
 	p.push_back (UserProperty (UserProperty::VIDEO, _("Length"), length (), _("video frames")));
-	p.push_back (UserProperty (UserProperty::VIDEO, _("Size"), String::compose ("%1x%2", size().width, size().height)));
+	if (auto s = size()) {
+		p.push_back(UserProperty(UserProperty::VIDEO, _("Size"), String::compose("%1x%2", s->width, s->height)));
+	}
 }
 
 void
@@ -643,7 +660,7 @@ VideoContent::modify_trim_start (ContentTime& trim) const
 
 
 /** @param film_container The size of the container for the DCP that we are working on */
-dcp::Size
+optional<dcp::Size>
 VideoContent::scaled_size (dcp::Size film_container)
 {
 	if (_custom_ratio) {
@@ -658,18 +675,22 @@ VideoContent::scaled_size (dcp::Size film_container)
 	}
 
 	auto size = size_after_crop ();
-	size.width = std::lrint(size.width * _sample_aspect_ratio.get_value_or(1));
+	if (!size) {
+		return {};
+	}
+
+	size->width = std::lrint(size->width * _sample_aspect_ratio.get_value_or(1));
 
 	/* This is what we will return unless there is any legacy stuff to take into account */
-	auto auto_size = fit_ratio_within (size.ratio(), film_container);
+	auto auto_size = fit_ratio_within(size->ratio(), film_container);
 
 	if (_legacy_ratio) {
 		if (fit_ratio_within(*_legacy_ratio, film_container) != auto_size) {
 			_custom_ratio = *_legacy_ratio;
-			_legacy_ratio = optional<float>();
+			_legacy_ratio = {};
 			return fit_ratio_within(*_custom_ratio, film_container);
 		}
-		_legacy_ratio = boost::optional<float>();
+		_legacy_ratio = {};
 	}
 
 	return _pixel_quanta.round (auto_size);
diff --git a/src/lib/video_content.h b/src/lib/video_content.h
index 9f6cc105e..e7e8eb1b3 100644
--- a/src/lib/video_content.h
+++ b/src/lib/video_content.h
@@ -85,7 +85,7 @@ public:
 		return _length;
 	}
 
-	dcp::Size size () const {
+	boost::optional<dcp::Size> size () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _size;
 	}
@@ -204,9 +204,9 @@ public:
 
 
 	/* XXX: names for these? */
-	dcp::Size size_after_3d_split () const;
-	dcp::Size size_after_crop () const;
-	dcp::Size scaled_size (dcp::Size container_size);
+	boost::optional<dcp::Size> size_after_3d_split() const;
+	boost::optional<dcp::Size> size_after_crop() const;
+	boost::optional<dcp::Size> scaled_size(dcp::Size container_size);
 
 	boost::optional<double> fade (std::shared_ptr<const Film> film, Frame) const;
 
@@ -237,7 +237,7 @@ private:
 	bool _use;
 	Frame _length;
 	boost::optional<ColourConversion> _colour_conversion;
-	dcp::Size _size;
+	boost::optional<dcp::Size> _size;
 	VideoFrameType _frame_type;
 	Crop _crop;
 	/** ratio to scale cropped image to (or none to guess); i.e. if set, scale to _custom_ratio:1 */
diff --git a/src/lib/video_examiner.h b/src/lib/video_examiner.h
index 44a1186bb..b670dafc5 100644
--- a/src/lib/video_examiner.h
+++ b/src/lib/video_examiner.h
@@ -41,8 +41,8 @@ public:
 
 	/** @return video frame rate (if known); must not be called if has_video() == false */
 	virtual boost::optional<double> video_frame_rate () const = 0;
-	/** @return video size; must not be called if has_video() == false */
-	virtual dcp::Size video_size () const = 0;
+	/** @return video size (if known) */
+	virtual boost::optional<dcp::Size> video_size() const = 0;
 	/** @return video length in frames; must not be called if has_video() == false */
 	virtual Frame video_length () const = 0;
 	/** @return video sample aspect ratio (if known); must not be called if has_video() == false */
diff --git a/src/lib/video_mxf_examiner.cc b/src/lib/video_mxf_examiner.cc
index 8c7029ea8..7a05f3369 100644
--- a/src/lib/video_mxf_examiner.cc
+++ b/src/lib/video_mxf_examiner.cc
@@ -48,7 +48,7 @@ VideoMXFExaminer::video_frame_rate () const
 	return _asset->frame_rate().as_float ();
 }
 
-dcp::Size
+optional<dcp::Size>
 VideoMXFExaminer::video_size () const
 {
 	return _asset->size ();
diff --git a/src/lib/video_mxf_examiner.h b/src/lib/video_mxf_examiner.h
index f5c9bf1fd..e4cd98f07 100644
--- a/src/lib/video_mxf_examiner.h
+++ b/src/lib/video_mxf_examiner.h
@@ -35,7 +35,7 @@ public:
 		return true;
 	}
 	boost::optional<double> video_frame_rate () const override;
-	dcp::Size video_size () const override;
+	boost::optional<dcp::Size> video_size() const override;
 	Frame video_length () const override;
 	boost::optional<double> sample_aspect_ratio () const override;
 	bool yuv () const override;