Fix incorrect container size when loading a VF/OV combination into the player.

diff --git a/ChangeLog b/ChangeLog
index 0f9f0ab51467ac3bce03ffed8434f45878035c08..304c9d3d8b314b2e59ea97347dbf1655e3a6ec00 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 2018-04-27  Carl Hetherington  <cth@carlh.net>
 
+       * Fix incorrect container size when loading a OV/VF combination into the player.
+
        * Fix obscured subtitles in some cases.
 
 2018-04-25  Carl Hetherington  <cth@carlh.net>

diff --git a/src/lib/audio_examiner.h b/src/lib/audio_examiner.h
index 0929c835752893b06e94f8b50773c2e015e0c672..a1d952c35ef6a97b125c6350d18b9f1725328617 100644 (file)
--- 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 b5209e2c761c4c4522a355acab7ce79486ec1f50..1dffa6b3682bda2685bc59be8b74764d2374cba9 100644 (file)
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -72,9 +72,6 @@ DCPContent::DCPContent (shared_ptr<const Film> 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<const Film> 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<int> ("AudioFrameRate"),
-                               /* AudioLength was not present in some old metadata versions */
-                               node->optional_number_child<Frame>("AudioLength").get_value_or (
-                                       video->length() * node->number_child<int>("AudioFrameRate") / video_frame_rate().get()
-                                       ),
-                               AudioMapping (node->node_child ("AudioMapping"), version)
+       if (video && audio) {
+               audio->set_stream (
+                       AudioStreamPtr (
+                               new AudioStream (
+                                       node->number_child<int> ("AudioFrameRate"),
+                                       /* AudioLength was not present in some old metadata versions */
+                                       node->optional_number_child<Frame>("AudioLength").get_value_or (
+                                               video->length() * node->number_child<int>("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> job)
        Content::examine (job);
 
        shared_ptr<DCPExaminer> 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> 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<UserProperty>& 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_ptr<ContentPart> (shared_ptr<const Co
 bool
 DCPContent::can_reference_video (string& why_not) const
 {
+       if (!video) {
+               why_not = _("There is no video in this DCP");
+               return false;
+       }
+
        if (film()->frame_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 0250fa9fef31a7a73fd969c9c0a6afd59f236de1..5f06501b621f2c369e1c8d8ed3b5e1d6a69df01d 100644 (file)
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -54,7 +54,9 @@ DCPDecoder::DCPDecoder (shared_ptr<const DCPContent> c, shared_ptr<Log> 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 80887a609eb01f9c593e5e1bfb4f4c7841a3cfe6..c097877a33816f02760491796598a1088793f1e1 100644 (file)
--- a/src/lib/dcp_examiner.cc
+++ b/src/lib/dcp_examiner.cc
@@ -55,6 +55,8 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> 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<const DCPContent> content)
                                throw DCPError (_("Mismatched frame rates in DCP"));
                        }
 
+                       _has_video = true;
                        shared_ptr<dcp::PictureAsset> asset = i->main_picture()->asset ();
                        if (!_video_size) {
                                _video_size = asset->size ();
@@ -137,6 +140,7 @@ DCPExaminer::DCPExaminer (shared_ptr<const DCPContent> content)
                                return;
                        }
 
+                       _has_audio = true;
                        shared_ptr<dcp::SoundAsset> asset = i->main_sound()->asset ();
 
                        if (!_audio_channels) {

diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h
index aaeec6d3249c6405bee61bda579527ddbf367105..9d6faa7e8d49ca230c55a0d7b179d6bc7f6d1a7d 100644 (file)
--- 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<const DCPContent>);
 
+       bool has_video () const {
+               return _has_video;
+       }
+
        boost::optional<double> 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<dcp::Standard> _standard;

diff --git a/src/lib/image_examiner.h b/src/lib/image_examiner.h
index 281b002f33572c1708dfef7f42297d8b14c9a31e..2e743a82e4b318b3450a0c5546d29fc97d70d3a8 100644 (file)
--- 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<const Film>, boost::shared_ptr<const ImageContent>, boost::shared_ptr<Job>);
 
+       bool has_video () const {
+               return true;
+       }
        boost::optional<double> 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 1ff16a704f233f5cddf27e7f6b826f7b1300e1df..dd5d08c9e75341773265f7fa94d3bc1af5a9355c 100644 (file)
--- 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<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 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<double> sample_aspect_ratio () const {
                return boost::optional<double> ();
        }
+       /** @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 3a0189a2fc3bc2a190e74efee52afd35bd6e5f45..205daa361c3652a41762183f9bf6e83afd768674 100644 (file)
--- 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<const VideoMXFContent>);
 
+       bool has_video () const {
+               return true;
+       }
        boost::optional<double> 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 19768e292242657457d9b74159bebb622688cb0f..2fe9aca8b83f585d88b0a73537c5b09467501207 100644 (file)
--- 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<Job>());
-                       /* 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<DCPContent> 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;