Fix failure to load OV after adding a VF to a project.

This has the same cause as
19f51503621a57794bd79bac053c9e6549a69f46
i.e. the DCPDecoder re-use optimisation.  This commit tries to
re-fix 19f515 in a more general way which also takes into account
the OV/VF bug.  It also adds a unit test.

diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc
index 95cad926620ad9a4824b40e8ff1914331d3e776e..4b189189e0b61fc395e83658968a1b34cf44947b 100644 (file)
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2014-2018 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2014-2019 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -28,6 +28,7 @@
 #include "ffmpeg_image_proxy.h"
 #include "image.h"
 #include "config.h"
+#include "digester.h"
 #include <dcp/dcp.h>
 #include <dcp/cpl.h>
 #include <dcp/reel.h>
@@ -52,6 +53,7 @@
 
 using std::list;
 using std::cout;
+using std::string;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
 using boost::optional;
@@ -75,16 +77,18 @@ DCPDecoder::DCPDecoder (shared_ptr<const Film> film, shared_ptr<const DCPContent
                }
        }
 
-       if (old) {
-               _reels = old->_reels;
+       /* We try to avoid re-scanning the DCP's files every time we make a new DCPDecoder; we do this
+          by re-using the _reels list.  Before we do this we need to check that nothing too serious
+          has changed in the DCPContent.
 
-               /* We might have gained a KDM since we made the Reel objects */
-               if (_dcp_content->kdm ()) {
-                       dcp::DecryptedKDM k = decrypted_kdm ();
-                       BOOST_FOREACH (shared_ptr<dcp::Reel> i, _reels) {
-                               i->add (k);
-                       }
-               }
+          We do this by storing a digest of the important bits of the DCPContent and then checking that's
+          the same before we re-use _reels.
+       */
+
+       _lazy_digest = calculate_lazy_digest (c);
+
+       if (old && old->lazy_digest() == _lazy_digest) {
+               _reels = old->_reels;
        } else {
 
                list<shared_ptr<dcp::CPL> > cpl_list = cpls ();
@@ -425,3 +429,18 @@ DCPDecoder::set_forced_reduction (optional<int> reduction)
 {
        _forced_reduction = reduction;
 }
+
+string
+DCPDecoder::calculate_lazy_digest (shared_ptr<const DCPContent> c) const
+{
+       Digester d;
+       BOOST_FOREACH (boost::filesystem::path i, c->paths()) {
+               d.add (i.string());
+       }
+       d.add (static_cast<bool>(_dcp_content->kdm()));
+       d.add (static_cast<bool>(c->cpl()));
+       if (c->cpl()) {
+               d.add (c->cpl().get());
+       }
+       return d.get ();
+}

diff --git a/src/lib/dcp_decoder.h b/src/lib/dcp_decoder.h
index 2e63b24a296ca488079150378a8942982cada924..496d95740d63cf26e9c74024dd43f506416cbf7b 100644 (file)
--- a/src/lib/dcp_decoder.h
+++ b/src/lib/dcp_decoder.h
@@ -58,6 +58,10 @@ public:
        bool pass ();
        void seek (dcpomatic::ContentTime t, bool accurate);
 
+       std::string lazy_digest () const {
+               return _lazy_digest;
+       }
+
 private:
        friend struct dcp_subtitle_within_dcp_test;
 
@@ -72,6 +76,7 @@ private:
                boost::shared_ptr<TextDecoder> decoder,
                dcp::Size size
                );
+       std::string calculate_lazy_digest (boost::shared_ptr<const DCPContent>) const;
 
        /** Time of next thing to return from pass relative to the start of _reel */
        dcpomatic::ContentTime _next;
@@ -89,4 +94,6 @@ private:
 
        bool _decode_referenced;
        boost::optional<int> _forced_reduction;
+
+       std::string _lazy_digest;
 };

diff --git a/src/lib/player.h b/src/lib/player.h
index 7558f6da0bd2fd62376ae2e666623239cefd1729..e99c345bb211ea99d683fcb5b3d32513305ad4c9 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -110,6 +110,7 @@ private:
        friend struct player_subframe_test;
        friend struct empty_test1;
        friend struct empty_test2;
+       friend struct check_reuse_old_data_test;
 
        void setup_pieces ();
        void setup_pieces_unlocked ();

diff --git a/test/dcp_decoder_test.cc b/test/dcp_decoder_test.cc
new file mode 100644 (file)
index 0000000..64d35a2
--- /dev/null
+++ b/test/dcp_decoder_test.cc
@@ -0,0 +1,140 @@
+/*
+    Copyright (C) 2019 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+/** @file  test/dcp_decoder_test.cc
+ *  @brief Test DCPDecoder class.
+ *  @ingroup selfcontained
+ */
+
+#include "lib/film.h"
+#include "lib/dcp_content.h"
+#include "lib/dcp_decoder.h"
+#include "lib/content_factory.h"
+#include "lib/player.h"
+#include "lib/examine_content_job.h"
+#include "lib/job_manager.h"
+#include "lib/config.h"
+#include "test.h"
+#include <dcp/dcp.h>
+#include <boost/test/unit_test.hpp>
+#include <iostream>
+
+using std::list;
+using std::string;
+using std::vector;
+using boost::shared_ptr;
+
+/* Check that DCPDecoder reuses old data when it should */
+BOOST_AUTO_TEST_CASE (check_reuse_old_data_test)
+{
+       /* Make some DCPs */
+
+       shared_ptr<Film> ov = new_test_film2 ("check_reuse_old_data_ov");
+       ov->examine_and_add_content (content_factory("test/data/flat_red.png").front());
+       BOOST_REQUIRE (!wait_for_jobs());
+       ov->make_dcp ();
+       BOOST_REQUIRE (!wait_for_jobs());
+
+       shared_ptr<Film> vf = new_test_film2 ("check_reuse_old_data_vf");
+       shared_ptr<DCPContent> ov_content(new DCPContent(ov->dir(ov->dcp_name(false))));
+       vf->examine_and_add_content (ov_content);
+       vf->examine_and_add_content (content_factory("test/data/L.wav").front());
+       BOOST_REQUIRE (!wait_for_jobs());
+       ov_content->set_reference_video (true);
+       vf->make_dcp ();
+       BOOST_REQUIRE (!wait_for_jobs());
+
+       shared_ptr<Film> encrypted = new_test_film2 ("check_reuse_old_data_decrypted");
+       encrypted->examine_and_add_content (content_factory("test/data/flat_red.png").front());
+       BOOST_REQUIRE (!wait_for_jobs());
+       encrypted->set_encrypted (true);
+       encrypted->make_dcp ();
+       BOOST_REQUIRE (!wait_for_jobs());
+
+       dcp::DCP encrypted_dcp (encrypted->dir(encrypted->dcp_name()));
+       encrypted_dcp.read ();
+
+       dcp::EncryptedKDM kdm = encrypted->make_kdm (
+               Config::instance()->decryption_chain()->leaf(),
+               vector<string>(),
+               encrypted_dcp.cpls().front()->file().get(),
+               dcp::LocalTime ("2014-07-21T00:00:00+00:00"),
+               dcp::LocalTime ("2024-07-21T00:00:00+00:00"),
+               dcp::MODIFIED_TRANSITIONAL_1,
+               true, 0
+               );
+
+
+       /* Add just the OV to a new project, move it around a bit and check that
+          the _reels get reused.
+       */
+       shared_ptr<Film> test = new_test_film2 ("check_reuse_old_data_test1");
+       ov_content.reset (new DCPContent(ov->dir(ov->dcp_name(false))));
+       test->examine_and_add_content (ov_content);
+       BOOST_REQUIRE (!wait_for_jobs());
+       shared_ptr<Player> player (new Player(test, test->playlist()));
+
+       shared_ptr<DCPDecoder> decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       list<shared_ptr<dcp::Reel> > reels = decoder->reels();
+
+       ov_content->set_position (test, dcpomatic::DCPTime(96000));
+       decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       BOOST_REQUIRE (reels == decoder->reels());
+
+       /* Add the VF to a new project, then add the OV and check that the
+          _reels did not get reused.
+       */
+       test = new_test_film2 ("check_reuse_old_data_test2");
+       shared_ptr<DCPContent> vf_content (new DCPContent(vf->dir(vf->dcp_name(false))));
+       test->examine_and_add_content (vf_content);
+       BOOST_REQUIRE (!wait_for_jobs());
+       player.reset (new Player(test, test->playlist()));
+
+       decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       reels = decoder->reels();
+
+       vf_content->add_ov (ov->dir(ov->dcp_name(false)));
+       JobManager::instance()->add (shared_ptr<Job>(new ExamineContentJob(test, vf_content)));
+       BOOST_REQUIRE (!wait_for_jobs());
+       decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       BOOST_REQUIRE (reels != decoder->reels());
+
+       /* Add a KDM to an encrypted DCP and check that the _reels did not get reused */
+       test = new_test_film2 ("check_reuse_old_data_test3");
+       shared_ptr<DCPContent> encrypted_content (new DCPContent(encrypted->dir(encrypted->dcp_name(false))));
+       test->examine_and_add_content (encrypted_content);
+       BOOST_REQUIRE (!wait_for_jobs());
+       player.reset (new Player(test, test->playlist()));
+
+       decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       reels = decoder->reels();
+
+       encrypted_content->add_kdm (kdm);
+       JobManager::instance()->add (shared_ptr<Job>(new ExamineContentJob(test, encrypted_content)));
+       BOOST_REQUIRE (!wait_for_jobs());
+       decoder = boost::dynamic_pointer_cast<DCPDecoder>(player->_pieces.front()->decoder);
+       BOOST_REQUIRE (decoder);
+       BOOST_REQUIRE (reels != decoder->reels());
+}

diff --git a/test/wscript b/test/wscript
index 1d2fafc02627c96a53a331d342c167fbaa4bec1c..3c6170dcf3ef2e4f9c4190b786e562544e68f4d4 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -61,6 +61,7 @@ def build(bld):
                  create_cli_test.cc
                  crypto_test.cc
                  dcpomatic_time_test.cc
+                 dcp_decoder_test.cc
                  dcp_playback_test.cc
                  dcp_subtitle_test.cc
                  digest_test.cc