From dd9f4f7e9511f8f830ec05d1b60c475c6b2d71e0 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 27 Apr 2018 15:13:42 +0100 Subject: [PATCH] Fix incorrect container size when loading a VF/OV combination into the player. --- ChangeLog | 2 + src/lib/audio_examiner.h | 1 + src/lib/dcp_content.cc | 98 ++++++++++++++++++++++++----------- src/lib/dcp_decoder.cc | 12 +++-- src/lib/dcp_examiner.cc | 4 ++ src/lib/dcp_examiner.h | 16 +++++- src/lib/image_examiner.h | 3 ++ src/lib/video_examiner.h | 8 +++ src/lib/video_mxf_examiner.h | 3 ++ src/tools/dcpomatic_player.cc | 30 ++++++----- 10 files changed, 131 insertions(+), 46 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0f9f0ab51..304c9d3d8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 2018-04-27 Carl Hetherington + * Fix incorrect container size when loading a OV/VF combination into the player. + * Fix obscured subtitles in some cases. 2018-04-25 Carl Hetherington diff --git a/src/lib/audio_examiner.h b/src/lib/audio_examiner.h index 0929c8357..a1d952c35 100644 --- a/src/lib/audio_examiner.h +++ b/src/lib/audio_examiner.h @@ -35,6 +35,7 @@ class AudioExaminer public: virtual ~AudioExaminer () {} + virtual bool has_audio () const = 0; virtual int audio_channels () const = 0; virtual Frame audio_length () const = 0; virtual int audio_frame_rate () const = 0; diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc index b5209e2c7..1dffa6b36 100644 --- a/src/lib/dcp_content.cc +++ b/src/lib/dcp_content.cc @@ -72,9 +72,6 @@ DCPContent::DCPContent (shared_ptr film, boost::filesystem::path p) , _reference_subtitle (false) , _three_d (false) { - video.reset (new VideoContent (this)); - audio.reset (new AudioContent (this)); - read_directory (p); set_default_colour_conversion (); } @@ -86,18 +83,20 @@ DCPContent::DCPContent (shared_ptr film, cxml::ConstNodePtr node, in audio = AudioContent::from_xml (this, node, version); subtitle = SubtitleContent::from_xml (this, node, version); - audio->set_stream ( - AudioStreamPtr ( - new AudioStream ( - node->number_child ("AudioFrameRate"), - /* AudioLength was not present in some old metadata versions */ - node->optional_number_child("AudioLength").get_value_or ( - video->length() * node->number_child("AudioFrameRate") / video_frame_rate().get() - ), - AudioMapping (node->node_child ("AudioMapping"), version) + if (video && audio) { + audio->set_stream ( + AudioStreamPtr ( + new AudioStream ( + node->number_child ("AudioFrameRate"), + /* AudioLength was not present in some old metadata versions */ + node->optional_number_child("AudioLength").get_value_or ( + video->length() * node->number_child("AudioFrameRate") / video_frame_rate().get() + ), + AudioMapping (node->node_child ("AudioMapping"), version) + ) ) - ) - ); + ); + } _name = node->string_child ("Name"); _encrypted = node->bool_child ("Encrypted"); @@ -152,15 +151,28 @@ DCPContent::examine (shared_ptr job) Content::examine (job); shared_ptr examiner (new DCPExaminer (shared_from_this ())); - video->take_from_examiner (examiner); - set_default_colour_conversion (); - AudioStreamPtr as (new AudioStream (examiner->audio_frame_rate(), examiner->audio_length(), examiner->audio_channels())); - audio->set_stream (as); - AudioMapping m = as->mapping (); - film()->make_audio_mapping_default (m); - as->set_mapping (m); - signal_changed (AudioContentProperty::STREAMS); + if (examiner->has_video()) { + { + boost::mutex::scoped_lock lm (_mutex); + video.reset (new VideoContent (this)); + } + video->take_from_examiner (examiner); + set_default_colour_conversion (); + } + + if (examiner->has_audio()) { + { + boost::mutex::scoped_lock lm (_mutex); + audio.reset (new AudioContent (this)); + } + AudioStreamPtr as (new AudioStream (examiner->audio_frame_rate(), examiner->audio_length(), examiner->audio_channels())); + audio->set_stream (as); + AudioMapping m = as->mapping (); + film()->make_audio_mapping_default (m); + as->set_mapping (m); + signal_changed (AudioContentProperty::STREAMS); + } bool has_subtitles = false; { @@ -199,7 +211,9 @@ DCPContent::examine (shared_ptr job) signal_changed (AudioContentProperty::STREAMS); - video->set_frame_type (_three_d ? VIDEO_FRAME_TYPE_3D : VIDEO_FRAME_TYPE_2D); + if (video) { + video->set_frame_type (_three_d ? VIDEO_FRAME_TYPE_3D : VIDEO_FRAME_TYPE_2D); + } } string @@ -212,9 +226,14 @@ DCPContent::summary () const string DCPContent::technical_summary () const { - return Content::technical_summary() + " - " - + video->technical_summary() + " - " - + audio->technical_summary() + " - "; + string s = Content::technical_summary() + " - "; + if (video) { + s += video->technical_summary() + " - "; + } + if (audio) { + s += audio->technical_summary() + " - "; + } + return s; } void @@ -274,6 +293,9 @@ DCPContent::as_xml (xmlpp::Node* node, bool with_paths) const DCPTime DCPContent::full_length () const { + if (!video) { + return DCPTime(); + } FrameRateChange const frc (active_video_frame_rate (), film()->video_frame_rate ()); return DCPTime::from_frames (llrint (video->length () * frc.factor ()), film()->video_frame_rate ()); } @@ -281,7 +303,12 @@ DCPContent::full_length () const string DCPContent::identifier () const { - string s = Content::identifier() + "_" + video->identifier() + "_"; + string s = Content::identifier() + "_"; + + if (video) { + s += video->identifier() + "_"; + } + if (subtitle) { s += subtitle->identifier () + " "; } @@ -332,15 +359,21 @@ void DCPContent::add_properties (list& p) const { Content::add_properties (p); - video->add_properties (p); - audio->add_properties (p); + if (video) { + video->add_properties (p); + } + if (audio) { + audio->add_properties (p); + } } void DCPContent::set_default_colour_conversion () { /* Default to no colour conversion for DCPs */ - video->unset_colour_conversion (); + if (video) { + video->unset_colour_conversion (); + } } void @@ -484,6 +517,11 @@ DCPContent::can_reference (function (shared_ptrframe_size() != video->size()) { /// TRANSLATORS: this string will follow "Cannot reference this DCP: " why_not = _("its video frame size differs from the film's."); diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc index 0250fa9fe..5f06501b6 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -54,7 +54,9 @@ DCPDecoder::DCPDecoder (shared_ptr c, shared_ptr log, boo : DCP (c) , _decode_referenced (false) { - video.reset (new VideoDecoder (this, c, log)); + if (c->video) { + video.reset (new VideoDecoder (this, c, log)); + } if (c->audio) { audio.reset (new AudioDecoder (this, c->audio, log, fast)); } @@ -304,8 +306,12 @@ DCPDecoder::set_decode_referenced (bool r) { _decode_referenced = r; - video->set_ignore (_dcp_content->reference_video() && !_decode_referenced); - audio->set_ignore (_dcp_content->reference_audio() && !_decode_referenced); + if (video) { + video->set_ignore (_dcp_content->reference_video() && !_decode_referenced); + } + if (audio) { + audio->set_ignore (_dcp_content->reference_audio() && !_decode_referenced); + } } void diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc index 80887a609..c097877a3 100644 --- a/src/lib/dcp_examiner.cc +++ b/src/lib/dcp_examiner.cc @@ -55,6 +55,8 @@ DCPExaminer::DCPExaminer (shared_ptr content) : DCP (content) , _video_length (0) , _audio_length (0) + , _has_video (false) + , _has_audio (false) , _has_subtitles (false) , _encrypted (false) , _needs_assets (false) @@ -120,6 +122,7 @@ DCPExaminer::DCPExaminer (shared_ptr content) throw DCPError (_("Mismatched frame rates in DCP")); } + _has_video = true; shared_ptr asset = i->main_picture()->asset (); if (!_video_size) { _video_size = asset->size (); @@ -137,6 +140,7 @@ DCPExaminer::DCPExaminer (shared_ptr content) return; } + _has_audio = true; shared_ptr asset = i->main_sound()->asset (); if (!_audio_channels) { diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h index aaeec6d32..9d6faa7e8 100644 --- a/src/lib/dcp_examiner.h +++ b/src/lib/dcp_examiner.h @@ -33,12 +33,18 @@ class DCPExaminer : public DCP, public VideoExaminer, public AudioExaminer public: explicit DCPExaminer (boost::shared_ptr); + bool has_video () const { + return _has_video; + } + boost::optional video_frame_rate () const { return _video_frame_rate; } dcp::Size video_size () const { - return _video_size.get_value_or (dcp::Size (1998, 1080)); + DCPOMATIC_ASSERT (_has_video); + DCPOMATIC_ASSERT (_video_size); + return *_video_size; } Frame video_length () const { @@ -65,6 +71,10 @@ public: return _needs_assets; } + bool has_audio () const { + return _has_audio; + } + int audio_channels () const { return _audio_channels.get_value_or (0); } @@ -107,6 +117,10 @@ private: std::string _name; bool _has_subtitles; bool _encrypted; + /** true if this DCP has video content (but false if it has unresolved references to video content) */ + bool _has_video; + /** true if this DCP has audio content (but false if it has unresolved references to audio content) */ + bool _has_audio; bool _needs_assets; bool _kdm_valid; boost::optional _standard; diff --git a/src/lib/image_examiner.h b/src/lib/image_examiner.h index 281b002f3..2e743a82e 100644 --- a/src/lib/image_examiner.h +++ b/src/lib/image_examiner.h @@ -27,6 +27,9 @@ class ImageExaminer : public VideoExaminer public: ImageExaminer (boost::shared_ptr, boost::shared_ptr, boost::shared_ptr); + bool has_video () const { + return true; + } boost::optional video_frame_rate () const; dcp::Size video_size () const; Frame video_length () const { diff --git a/src/lib/video_examiner.h b/src/lib/video_examiner.h index 1ff16a704..dd5d08c9e 100644 --- a/src/lib/video_examiner.h +++ b/src/lib/video_examiner.h @@ -33,11 +33,19 @@ class VideoExaminer { public: virtual ~VideoExaminer () {} + + virtual bool has_video () const = 0; + + /** @return video frame rate (if known); must not be called if has_video() == false */ virtual boost::optional 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 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 */ virtual boost::optional sample_aspect_ratio () const { return boost::optional (); } + /** @return true if this video is in YUV; must not be called if has_video() == false */ virtual bool yuv () const = 0; }; diff --git a/src/lib/video_mxf_examiner.h b/src/lib/video_mxf_examiner.h index 3a0189a2f..205daa361 100644 --- a/src/lib/video_mxf_examiner.h +++ b/src/lib/video_mxf_examiner.h @@ -31,6 +31,9 @@ class VideoMXFExaminer : public VideoExaminer public: explicit VideoMXFExaminer (boost::shared_ptr); + bool has_video () const { + return true; + } boost::optional video_frame_rate () const; dcp::Size video_size () const; Frame video_length () const; diff --git a/src/tools/dcpomatic_player.cc b/src/tools/dcpomatic_player.cc index 19768e292..2fe9aca8b 100644 --- a/src/tools/dcpomatic_player.cc +++ b/src/tools/dcpomatic_player.cc @@ -213,19 +213,12 @@ public: return; } - if (dcp->subtitle) { - dcp->subtitle->set_use (true); - } + setup_from_dcp (dcp); if (dcp->three_d()) { _film->set_three_d (true); } - Ratio const * r = Ratio::nearest_from_ratio(dcp->video->size().ratio()); - if (r) { - _film->set_container(r); - } - _viewer->set_film (_film); _viewer->set_position (DCPTime ()); _info->triggered_update (); @@ -364,10 +357,7 @@ private: DCPOMATIC_ASSERT (dcp); dcp->add_ov (wx_to_std(c->GetPath())); dcp->examine (shared_ptr()); - /* Maybe we just gained some subtitles */ - if (dcp->subtitle) { - dcp->subtitle->set_use (true); - } + setup_from_dcp (dcp); } c->Destroy (); @@ -594,6 +584,22 @@ private: _viewer->forward_frame (); } +private: + + void setup_from_dcp (shared_ptr dcp) + { + if (dcp->subtitle) { + dcp->subtitle->set_use (true); + } + + if (dcp->video) { + Ratio const * r = Ratio::nearest_from_ratio(dcp->video->size().ratio()); + if (r) { + _film->set_container(r); + } + } + } + bool _update_news_requested; PlayerInformation* _info; wxPreferencesEditor* _config_dialog; -- 2.30.2