Make similar changes to the previous commit for _xml_id.
[libdcp.git] / src / verify.cc
index 0082cfcdbd8a22191194af58ce12f2a3a175eb5e..5550a67f21867fa9f99efa843c02d6a1cbeacf39 100644 (file)
  */
 
 
-#include "verify.h"
-#include "dcp.h"
+#include "compose.hpp"
 #include "cpl.h"
+#include "dcp.h"
+#include "exceptions.h"
+#include "interop_subtitle_asset.h"
+#include "mono_picture_asset.h"
+#include "mono_picture_frame.h"
+#include "raw_convert.h"
 #include "reel.h"
 #include "reel_closed_caption_asset.h"
+#include "reel_interop_subtitle_asset.h"
+#include "reel_markers_asset.h"
 #include "reel_picture_asset.h"
 #include "reel_sound_asset.h"
+#include "reel_smpte_subtitle_asset.h"
 #include "reel_subtitle_asset.h"
-#include "interop_subtitle_asset.h"
-#include "mono_picture_asset.h"
-#include "mono_picture_frame.h"
+#include "smpte_subtitle_asset.h"
 #include "stereo_picture_asset.h"
 #include "stereo_picture_frame.h"
-#include "exceptions.h"
-#include "compose.hpp"
-#include "raw_convert.h"
-#include "reel_markers_asset.h"
-#include "smpte_subtitle_asset.h"
-#include <xercesc/util/PlatformUtils.hpp>
-#include <xercesc/parsers/XercesDOMParser.hpp>
-#include <xercesc/parsers/AbstractDOMParser.hpp>
-#include <xercesc/sax/HandlerBase.hpp>
+#include "verify.h"
+#include "verify_j2k.h"
+#include <xercesc/dom/DOMAttr.hpp>
+#include <xercesc/dom/DOMDocument.hpp>
+#include <xercesc/dom/DOMError.hpp>
+#include <xercesc/dom/DOMErrorHandler.hpp>
+#include <xercesc/dom/DOMException.hpp>
 #include <xercesc/dom/DOMImplementation.hpp>
 #include <xercesc/dom/DOMImplementationLS.hpp>
 #include <xercesc/dom/DOMImplementationRegistry.hpp>
 #include <xercesc/dom/DOMLSParser.hpp>
-#include <xercesc/dom/DOMException.hpp>
-#include <xercesc/dom/DOMDocument.hpp>
-#include <xercesc/dom/DOMNodeList.hpp>
-#include <xercesc/dom/DOMError.hpp>
 #include <xercesc/dom/DOMLocator.hpp>
 #include <xercesc/dom/DOMNamedNodeMap.hpp>
-#include <xercesc/dom/DOMAttr.hpp>
-#include <xercesc/dom/DOMErrorHandler.hpp>
+#include <xercesc/dom/DOMNodeList.hpp>
 #include <xercesc/framework/LocalFileInputSource.hpp>
 #include <xercesc/framework/MemBufInputSource.hpp>
+#include <xercesc/parsers/AbstractDOMParser.hpp>
+#include <xercesc/parsers/XercesDOMParser.hpp>
+#include <xercesc/sax/HandlerBase.hpp>
+#include <xercesc/util/PlatformUtils.hpp>
 #include <boost/algorithm/string.hpp>
+#include <iostream>
 #include <map>
 #include <vector>
-#include <iostream>
 
 
 using std::list;
@@ -153,22 +156,22 @@ private:
 class DCPErrorHandler : public ErrorHandler
 {
 public:
-       void warning(const SAXParseException& e)
+       void warning(const SAXParseException& e) override
        {
                maybe_add (XMLValidationError(e));
        }
 
-       void error(const SAXParseException& e)
+       void error(const SAXParseException& e) override
        {
                maybe_add (XMLValidationError(e));
        }
 
-       void fatalError(const SAXParseException& e)
+       void fatalError(const SAXParseException& e) override
        {
                maybe_add (XMLValidationError(e));
        }
 
-       void resetErrors() {
+       void resetErrors() override {
                _errors.clear ();
        }
 
@@ -243,7 +246,7 @@ public:
                add("http://www.smpte-ra.org/schemas/429-10/2008/Main-Stereo-Picture-CPL", "SMPTE-429-10-2008.xsd");
        }
 
-       InputSource* resolveEntity(XMLCh const *, XMLCh const * system_id)
+       InputSource* resolveEntity(XMLCh const *, XMLCh const * system_id) override
        {
                if (!system_id) {
                        return 0;
@@ -420,67 +423,60 @@ verify_language_tag (string tag, vector<VerificationNote>& notes)
 }
 
 
-enum class VerifyPictureAssetResult
-{
-       GOOD,
-       FRAME_NEARLY_TOO_LARGE,
-       BAD,
-};
-
-
-int
-biggest_frame_size (shared_ptr<const MonoPictureFrame> frame)
-{
-       return frame->size ();
-}
-
-int
-biggest_frame_size (shared_ptr<const StereoPictureFrame> frame)
+static void
+verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, boost::filesystem::path file, vector<VerificationNote>& notes, function<void (float)> progress)
 {
-       return max(frame->left()->size(), frame->right()->size());
-}
+       int biggest_frame = 0;
+       auto asset = dynamic_pointer_cast<PictureAsset>(reel_file_asset->asset_ref().asset());
+       auto const duration = asset->intrinsic_duration ();
 
+       auto check_and_add = [&notes](vector<VerificationNote> const& j2k_notes) {
+               for (auto i: j2k_notes) {
+                       if (find(notes.begin(), notes.end(), i) == notes.end()) {
+                               notes.push_back (i);
+                       }
+               }
+       };
 
-template <class A, class R, class F>
-optional<VerifyPictureAssetResult>
-verify_picture_asset_type (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
-{
-       auto asset = dynamic_pointer_cast<A>(reel_file_asset->asset_ref().asset());
-       if (!asset) {
-               return optional<VerifyPictureAssetResult>();
-       }
+       if (auto mono_asset = dynamic_pointer_cast<MonoPictureAsset>(reel_file_asset->asset_ref().asset())) {
+               auto reader = mono_asset->start_read ();
+               for (int64_t i = 0; i < duration; ++i) {
+                       auto frame = reader->get_frame (i);
+                       biggest_frame = max(biggest_frame, frame->size());
+                       if (!mono_asset->encrypted() || mono_asset->key()) {
+                               vector<VerificationNote> j2k_notes;
+                               verify_j2k (frame, j2k_notes);
+                               check_and_add (j2k_notes);
+                       }
+                       progress (float(i) / duration);
+               }
+       } else if (auto stereo_asset = dynamic_pointer_cast<StereoPictureAsset>(asset)) {
+               auto reader = stereo_asset->start_read ();
+               for (int64_t i = 0; i < duration; ++i) {
+                       auto frame = reader->get_frame (i);
+                       biggest_frame = max(biggest_frame, max(frame->left()->size(), frame->right()->size()));
+                       if (!stereo_asset->encrypted() || mono_asset->key()) {
+                               vector<VerificationNote> j2k_notes;
+                               verify_j2k (frame->left(), j2k_notes);
+                               verify_j2k (frame->right(), j2k_notes);
+                               check_and_add (j2k_notes);
+                       }
+                       progress (float(i) / duration);
+               }
 
-       int biggest_frame = 0;
-       auto reader = asset->start_read ();
-       auto const duration = asset->intrinsic_duration ();
-       for (int64_t i = 0; i < duration; ++i) {
-               shared_ptr<const F> frame = reader->get_frame (i);
-               biggest_frame = max(biggest_frame, biggest_frame_size(frame));
-               progress (float(i) / duration);
        }
 
        static const int max_frame =   rint(250 * 1000000 / (8 * asset->edit_rate().as_float()));
        static const int risky_frame = rint(230 * 1000000 / (8 * asset->edit_rate().as_float()));
        if (biggest_frame > max_frame) {
-               return VerifyPictureAssetResult::BAD;
+               notes.push_back ({
+                       VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+               });
        } else if (biggest_frame > risky_frame) {
-               return VerifyPictureAssetResult::FRAME_NEARLY_TOO_LARGE;
-       }
-
-       return VerifyPictureAssetResult::GOOD;
-}
-
-
-static VerifyPictureAssetResult
-verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, function<void (float)> progress)
-{
-       auto r = verify_picture_asset_type<MonoPictureAsset, MonoPictureAssetReader, MonoPictureFrame>(reel_file_asset, progress);
-       if (!r) {
-               r = verify_picture_asset_type<StereoPictureAsset, StereoPictureAssetReader, StereoPictureFrame>(reel_file_asset, progress);
+               notes.push_back ({
+                       VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+               });
        }
-
-       DCP_ASSERT (r);
-       return *r;
 }
 
 
@@ -512,21 +508,7 @@ verify_main_picture_asset (
                        break;
        }
        stage ("Checking picture frame sizes", asset->file());
-       auto const pr = verify_picture_asset (reel_asset, progress);
-       switch (pr) {
-               case VerifyPictureAssetResult::BAD:
-                       notes.push_back ({
-                               VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
-                       });
-                       break;
-               case VerifyPictureAssetResult::FRAME_NEARLY_TOO_LARGE:
-                       notes.push_back ({
-                               VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
-                       });
-                       break;
-               default:
-                       break;
-       }
+       verify_picture_asset (reel_asset, file, notes, progress);
 
        /* Only flat/scope allowed by Bv2.1 */
        if (
@@ -606,7 +588,9 @@ verify_main_sound_asset (
 
        stage ("Checking sound asset metadata", asset->file());
 
-       verify_language_tag (asset->language(), notes);
+       if (auto lang = asset->language()) {
+               verify_language_tag (*lang, notes);
+       }
        if (asset->sampling_rate() != 48000) {
                notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_SOUND_FRAME_RATE, raw_convert<string>(asset->sampling_rate()), *asset->file()});
        }
@@ -651,31 +635,27 @@ struct State
 };
 
 
-
+/** Verify stuff that is common to both subtitles and closed captions */
 void
-verify_smpte_subtitle_asset (
+verify_smpte_timed_text_asset (
        shared_ptr<const SMPTESubtitleAsset> asset,
-       vector<VerificationNote>& notes,
-       State& state
+       optional<int64_t> reel_asset_duration,
+       vector<VerificationNote>& notes
        )
 {
        if (asset->language()) {
-               auto const language = *asset->language();
-               verify_language_tag (language, notes);
-               if (!state.subtitle_language) {
-                       state.subtitle_language = language;
-               } else if (state.subtitle_language != language) {
-                       notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_SUBTITLE_LANGUAGES });
-               }
+               verify_language_tag (*asset->language(), notes);
        } else {
                notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE, *asset->file() });
        }
+
        auto const size = boost::filesystem::file_size(asset->file().get());
        if (size > 115 * 1024 * 1024) {
                notes.push_back (
                        { VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_TIMED_TEXT_SIZE_IN_BYTES, raw_convert<string>(size), *asset->file() }
                        );
        }
+
        /* XXX: I'm not sure what Bv2.1_7.2.1 means when it says "the font resource shall not be larger than 10MB"
         * but I'm hoping that checking for the total size of all fonts being <= 10MB will do.
         */
@@ -693,12 +673,56 @@ verify_smpte_subtitle_asset (
        } else if (asset->start_time() != Time()) {
                notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_SUBTITLE_START_TIME, asset->file().get() });
        }
+
+       if (reel_asset_duration && *reel_asset_duration != asset->intrinsic_duration()) {
+               notes.push_back (
+                       {
+                               VerificationNote::Type::BV21_ERROR,
+                               VerificationNote::Code::MISMATCHED_TIMED_TEXT_DURATION,
+                               String::compose("%1 %2", *reel_asset_duration, asset->intrinsic_duration()),
+                               asset->file().get()
+                       });
+       }
+}
+
+
+/** Verify SMPTE subtitle-only stuff */
+void
+verify_smpte_subtitle_asset (
+       shared_ptr<const SMPTESubtitleAsset> asset,
+       vector<VerificationNote>& notes,
+       State& state
+       )
+{
+       if (asset->language()) {
+               if (!state.subtitle_language) {
+                       state.subtitle_language = *asset->language();
+               } else if (state.subtitle_language != *asset->language()) {
+                       notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_SUBTITLE_LANGUAGES });
+               }
+       }
+
+       DCP_ASSERT (asset->resource_id());
+       auto xml_id = asset->xml_id();
+       if (xml_id) {
+               if (asset->resource_id().get() != xml_id) {
+                       notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISMATCHED_TIMED_TEXT_RESOURCE_ID });
+               }
+
+               if (asset->id() == asset->resource_id().get() || asset->id() == xml_id) {
+                       notes.push_back ({ VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INCORRECT_TIMED_TEXT_ASSET_ID });
+               }
+       } else {
+               notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSED_CHECK_OF_ENCRYPTED});
+       }
 }
 
 
+/** Verify all subtitle stuff */
 static void
 verify_subtitle_asset (
        shared_ptr<const SubtitleAsset> asset,
+       optional<int64_t> reel_asset_duration,
        function<void (string, optional<boost::filesystem::path>)> stage,
        boost::filesystem::path xsd_dtd_directory,
        vector<VerificationNote>& notes,
@@ -709,28 +733,47 @@ verify_subtitle_asset (
        /* Note: we must not use SubtitleAsset::xml_as_string() here as that will mean the data on disk
         * gets passed through libdcp which may clean up and therefore hide errors.
         */
-       validate_xml (asset->raw_xml(), xsd_dtd_directory, notes);
+       if (asset->raw_xml()) {
+               validate_xml (asset->raw_xml().get(), xsd_dtd_directory, notes);
+       } else {
+               notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSED_CHECK_OF_ENCRYPTED});
+       }
 
        auto smpte = dynamic_pointer_cast<const SMPTESubtitleAsset>(asset);
        if (smpte) {
+               verify_smpte_timed_text_asset (smpte, reel_asset_duration, notes);
                verify_smpte_subtitle_asset (smpte, notes, state);
        }
 }
 
 
+/** Verify all closed caption stuff */
 static void
 verify_closed_caption_asset (
        shared_ptr<const SubtitleAsset> asset,
+       optional<int64_t> reel_asset_duration,
        function<void (string, optional<boost::filesystem::path>)> stage,
        boost::filesystem::path xsd_dtd_directory,
-       vector<VerificationNote>& notes,
-       State& state
+       vector<VerificationNote>& notes
        )
 {
-       verify_subtitle_asset (asset, stage, xsd_dtd_directory, notes, state);
+       stage ("Checking closed caption XML", asset->file());
+       /* Note: we must not use SubtitleAsset::xml_as_string() here as that will mean the data on disk
+        * gets passed through libdcp which may clean up and therefore hide errors.
+        */
+       auto raw_xml = asset->raw_xml();
+       if (raw_xml) {
+               validate_xml (*raw_xml, xsd_dtd_directory, notes);
+               if (raw_xml->size() > 256 * 1024) {
+                       notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES, raw_convert<string>(raw_xml->size()), *asset->file()});
+               }
+       } else {
+               notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSED_CHECK_OF_ENCRYPTED});
+       }
 
-       if (asset->raw_xml().size() > 256 * 1024) {
-               notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES, raw_convert<string>(asset->raw_xml().size()), *asset->file()});
+       auto smpte = dynamic_pointer_cast<const SMPTESubtitleAsset>(asset);
+       if (smpte) {
+               verify_smpte_timed_text_asset (smpte, reel_asset_duration, notes);
        }
 }
 
@@ -739,10 +782,10 @@ static
 void
 verify_text_timing (
        vector<shared_ptr<Reel>> reels,
-       optional<int> picture_frame_rate,
+       int edit_rate,
        vector<VerificationNote>& notes,
        std::function<bool (shared_ptr<Reel>)> check,
-       std::function<string (shared_ptr<Reel>)> xml,
+       std::function<optional<string> (shared_ptr<Reel>)> xml,
        std::function<int64_t (shared_ptr<Reel>)> duration
        )
 {
@@ -751,32 +794,39 @@ verify_text_timing (
        auto too_short = false;
        auto too_close = false;
        auto too_early = false;
+       auto reel_overlap = false;
        /* current reel start time (in editable units) */
        int64_t reel_offset = 0;
 
-       std::function<void (cxml::ConstNodePtr, int, int, bool)> parse;
-       parse = [&parse, &last_out, &too_short, &too_close, &too_early, &reel_offset](cxml::ConstNodePtr node, int tcr, int pfr, bool first_reel) {
+       std::function<void (cxml::ConstNodePtr, optional<int>, optional<Time>, int, bool)> parse;
+       parse = [&parse, &last_out, &too_short, &too_close, &too_early, &reel_offset](cxml::ConstNodePtr node, optional<int> tcr, optional<Time> start_time, int er, bool first_reel) {
                if (node->name() == "Subtitle") {
                        Time in (node->string_attribute("TimeIn"), tcr);
+                       if (start_time) {
+                               in -= *start_time;
+                       }
                        Time out (node->string_attribute("TimeOut"), tcr);
-                       if (first_reel && in < Time(0, 0, 4, 0, tcr)) {
+                       if (start_time) {
+                               out -= *start_time;
+                       }
+                       if (first_reel && tcr && in < Time(0, 0, 4, 0, *tcr)) {
                                too_early = true;
                        }
                        auto length = out - in;
-                       if (length.as_editable_units(pfr) < 15) {
+                       if (length.as_editable_units_ceil(er) < 15) {
                                too_short = true;
                        }
                        if (last_out) {
                                /* XXX: this feels dubious - is it really what Bv2.1 means? */
-                               auto distance = reel_offset + in.as_editable_units(pfr) - *last_out;
+                               auto distance = reel_offset + in.as_editable_units_ceil(er) - *last_out;
                                if (distance >= 0 && distance < 2) {
                                        too_close = true;
                                }
                        }
-                       last_out = reel_offset + out.as_editable_units(pfr);
+                       last_out = reel_offset + out.as_editable_units_floor(er);
                } else {
                        for (auto i: node->node_children()) {
-                               parse(i, tcr, pfr, first_reel);
+                               parse(i, tcr, start_time, er, first_reel);
                        }
                }
        };
@@ -786,15 +836,41 @@ verify_text_timing (
                        continue;
                }
 
+               auto reel_xml = xml(reels[i]);
+               if (!reel_xml) {
+                       notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSED_CHECK_OF_ENCRYPTED});
+                       continue;
+               }
+
                /* We need to look at <Subtitle> instances in the XML being checked, so we can't use the subtitles
                 * read in by libdcp's parser.
                 */
 
-               auto doc = make_shared<cxml::Document>("SubtitleReel");
-               doc->read_string (xml(reels[i]));
-               auto const tcr = doc->number_child<int>("TimeCodeRate");
-               parse (doc, tcr, picture_frame_rate.get_value_or(24), i == 0);
-               reel_offset += duration(reels[i]);
+               shared_ptr<cxml::Document> doc;
+               optional<int> tcr;
+               optional<Time> start_time;
+               try {
+                       doc = make_shared<cxml::Document>("SubtitleReel");
+                       doc->read_string (*reel_xml);
+                       tcr = doc->number_child<int>("TimeCodeRate");
+                       auto start_time_string = doc->optional_string_child("StartTime");
+                       if (start_time_string) {
+                               start_time = Time(*start_time_string, tcr);
+                       }
+               } catch (...) {
+                       doc = make_shared<cxml::Document>("DCSubtitle");
+                       doc->read_string (*reel_xml);
+               }
+               parse (doc, tcr, start_time, edit_rate, i == 0);
+               auto end = reel_offset + duration(reels[i]);
+               if (last_out && *last_out > end) {
+                       reel_overlap = true;
+               }
+               reel_offset = end;
+       }
+
+       if (last_out && *last_out > reel_offset) {
+               reel_overlap = true;
        }
 
        if (too_early) {
@@ -814,6 +890,12 @@ verify_text_timing (
                        VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_SPACING
                });
        }
+
+       if (reel_overlap) {
+               notes.push_back ({
+                       VerificationNote::Type::ERROR, VerificationNote::Code::SUBTITLE_OVERLAPS_REEL_BOUNDARY
+               });
+       }
 }
 
 
@@ -924,18 +1006,19 @@ verify_text_timing (vector<shared_ptr<Reel>> reels, vector<VerificationNote>& no
                return;
        }
 
-       optional<int> picture_frame_rate;
-       if (reels[0]->main_picture()) {
-               picture_frame_rate = reels[0]->main_picture()->frame_rate().numerator;
-       }
-
        if (reels[0]->main_subtitle()) {
-               verify_text_timing (reels, picture_frame_rate, notes,
+               verify_text_timing (reels, reels[0]->main_subtitle()->edit_rate().numerator, notes,
                        [](shared_ptr<Reel> reel) {
                                return static_cast<bool>(reel->main_subtitle());
                        },
                        [](shared_ptr<Reel> reel) {
-                               return reel->main_subtitle()->asset()->raw_xml();
+                               auto interop = dynamic_pointer_cast<ReelInteropSubtitleAsset>(reel->main_subtitle());
+                               if (interop) {
+                                       return interop->asset()->raw_xml();
+                               }
+                               auto smpte = dynamic_pointer_cast<ReelSMPTESubtitleAsset>(reel->main_subtitle());
+                               DCP_ASSERT (smpte);
+                               return smpte->asset()->raw_xml();
                        },
                        [](shared_ptr<Reel> reel) {
                                return reel->main_subtitle()->actual_duration();
@@ -944,7 +1027,7 @@ verify_text_timing (vector<shared_ptr<Reel>> reels, vector<VerificationNote>& no
        }
 
        for (auto i = 0U; i < reels[0]->closed_captions().size(); ++i) {
-               verify_text_timing (reels, picture_frame_rate, notes,
+               verify_text_timing (reels, reels[0]->closed_captions()[i]->edit_rate().numerator, notes,
                        [i](shared_ptr<Reel> reel) {
                                return i < reel->closed_captions().size();
                        },
@@ -1024,14 +1107,9 @@ pkl_has_encrypted_assets (shared_ptr<DCP> dcp, shared_ptr<PKL> pkl)
        for (auto i: dcp->cpls()) {
                for (auto j: i->reel_file_assets()) {
                        if (j->asset_ref().resolved()) {
-                               /* It's a bit surprising / broken but Interop subtitle assets are represented
-                                * in reels by ReelSubtitleAsset which inherits ReelFileAsset, so it's possible for
-                                * ReelFileAssets to have assets which are not MXFs.
-                                */
-                               if (auto asset = dynamic_pointer_cast<MXF>(j->asset_ref().asset())) {
-                                       if (asset->encrypted()) {
-                                               encrypted.push_back(j->asset_ref().id());
-                                       }
+                               auto mxf = dynamic_pointer_cast<MXF>(j->asset_ref().asset());
+                               if (mxf && mxf->encrypted()) {
+                                       encrypted.push_back(j->asset_ref().id());
                                }
                        }
                }
@@ -1052,23 +1130,30 @@ dcp::verify (
        vector<boost::filesystem::path> directories,
        function<void (string, optional<boost::filesystem::path>)> stage,
        function<void (float)> progress,
-       boost::filesystem::path xsd_dtd_directory
+       optional<boost::filesystem::path> xsd_dtd_directory
        )
 {
-       xsd_dtd_directory = boost::filesystem::canonical (xsd_dtd_directory);
+       if (!xsd_dtd_directory) {
+               xsd_dtd_directory = resources_directory() / "xsd";
+       }
+       *xsd_dtd_directory = boost::filesystem::canonical (*xsd_dtd_directory);
 
        vector<VerificationNote> notes;
        State state{};
 
        vector<shared_ptr<DCP>> dcps;
        for (auto i: directories) {
-               dcps.push_back (shared_ptr<DCP> (new DCP (i)));
+               dcps.push_back (make_shared<DCP>(i));
        }
 
        for (auto dcp: dcps) {
                stage ("Checking DCP", dcp->directory());
+               bool carry_on = true;
                try {
-                       dcp->read (&notes);
+                       dcp->read (&notes, true);
+               } catch (MissingAssetmapError& e) {
+                       notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
+                       carry_on = false;
                } catch (ReadError& e) {
                        notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
                } catch (XMLError& e) {
@@ -1079,13 +1164,17 @@ dcp::verify (
                        notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::FAILED_READ, string(e.what())});
                }
 
+               if (!carry_on) {
+                       continue;
+               }
+
                if (dcp->standard() != Standard::SMPTE) {
                        notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::INVALID_STANDARD});
                }
 
                for (auto cpl: dcp->cpls()) {
                        stage ("Checking CPL", cpl->file());
-                       validate_xml (cpl->file().get(), xsd_dtd_directory, notes);
+                       validate_xml (cpl->file().get(), *xsd_dtd_directory, notes);
 
                        if (cpl->any_encrypted() && !cpl->all_encrypted()) {
                                notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::PARTIALLY_ENCRYPTED});
@@ -1167,7 +1256,7 @@ dcp::verify (
                                                notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_INTRINSIC_DURATION, i->id()});
                                        }
                                        auto file_asset = dynamic_pointer_cast<ReelFileAsset>(i);
-                                       if (file_asset && !file_asset->hash()) {
+                                       if (i->encryptable() && !file_asset->hash()) {
                                                notes.push_back ({VerificationNote::Type::BV21_ERROR, VerificationNote::Code::MISSING_HASH, i->id()});
                                        }
                                }
@@ -1214,7 +1303,7 @@ dcp::verify (
                                if (reel->main_subtitle()) {
                                        verify_main_subtitle_reel (reel->main_subtitle(), notes);
                                        if (reel->main_subtitle()->asset_ref().resolved()) {
-                                               verify_subtitle_asset (reel->main_subtitle()->asset(), stage, xsd_dtd_directory, notes, state);
+                                               verify_subtitle_asset (reel->main_subtitle()->asset(), reel->main_subtitle()->duration(), stage, *xsd_dtd_directory, notes, state);
                                        }
                                        have_main_subtitle = true;
                                } else {
@@ -1224,7 +1313,7 @@ dcp::verify (
                                for (auto i: reel->closed_captions()) {
                                        verify_closed_caption_reel (i, notes);
                                        if (i->asset_ref().resolved()) {
-                                               verify_closed_caption_asset (i->asset(), stage, xsd_dtd_directory, notes, state);
+                                               verify_closed_caption_asset (i->asset(), i->duration(), stage, *xsd_dtd_directory, notes);
                                        }
                                }
 
@@ -1238,6 +1327,8 @@ dcp::verify (
                                most_closed_captions = std::max (most_closed_captions, reel->closed_captions().size());
                        }
 
+                       verify_text_timing (cpl->reels(), notes);
+
                        if (dcp->standard() == Standard::SMPTE) {
 
                                if (have_main_subtitle && have_no_main_subtitle) {
@@ -1268,14 +1359,12 @@ dcp::verify (
                                if (lfoc == markers_seen.end()) {
                                        notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSING_LFOC});
                                } else {
-                                       auto lfoc_time = lfoc->second.as_editable_units(lfoc->second.tcr);
+                                       auto lfoc_time = lfoc->second.as_editable_units_ceil(lfoc->second.tcr);
                                        if (lfoc_time != (cpl->reels().back()->duration() - 1)) {
                                                notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INCORRECT_LFOC, raw_convert<string>(lfoc_time)});
                                        }
                                }
 
-                               verify_text_timing (cpl->reels(), notes);
-
                                LinesCharactersResult result;
                                for (auto reel: cpl->reels()) {
                                        if (reel->main_subtitle() && reel->main_subtitle()->asset()) {
@@ -1332,7 +1421,7 @@ dcp::verify (
 
                for (auto pkl: dcp->pkls()) {
                        stage ("Checking PKL", pkl->file());
-                       validate_xml (pkl->file().get(), xsd_dtd_directory, notes);
+                       validate_xml (pkl->file().get(), *xsd_dtd_directory, notes);
                        if (pkl_has_encrypted_assets(dcp, pkl)) {
                                cxml::Document doc ("PackingList");
                                doc.read_file (pkl->file().get());
@@ -1344,7 +1433,7 @@ dcp::verify (
 
                if (dcp->asset_map_path()) {
                        stage ("Checking ASSETMAP", dcp->asset_map_path().get());
-                       validate_xml (dcp->asset_map_path().get(), xsd_dtd_directory, notes);
+                       validate_xml (dcp->asset_map_path().get(), *xsd_dtd_directory, notes);
                } else {
                        notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::MISSING_ASSETMAP});
                }
@@ -1393,15 +1482,17 @@ dcp::note_to_string (VerificationNote note)
        case VerificationNote::Code::MISSING_ASSETMAP:
                return "No ASSETMAP or ASSETMAP.xml was found.";
        case VerificationNote::Code::INVALID_INTRINSIC_DURATION:
-               return String::compose("The intrinsic duration of the asset %1 is less than 1 second long.", note.note().get());
+               return String::compose("The intrinsic duration of the asset %1 is less than 1 second.", note.note().get());
        case VerificationNote::Code::INVALID_DURATION:
-               return String::compose("The duration of the asset %1 is less than 1 second long.", note.note().get());
+               return String::compose("The duration of the asset %1 is less than 1 second.", note.note().get());
        case VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES:
                return String::compose("The instantaneous bit rate of the picture asset %1 is larger than the limit of 250Mbit/s in at least one place.", note.file()->filename());
        case VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES:
                return String::compose("The instantaneous bit rate of the picture asset %1 is close to the limit of 250Mbit/s in at least one place.", note.file()->filename());
        case VerificationNote::Code::EXTERNAL_ASSET:
                return String::compose("The asset %1 that this DCP refers to is not included in the DCP.  It may be a VF.", note.note().get());
+       case VerificationNote::Code::THREED_ASSET_MARKED_AS_TWOD:
+               return String::compose("The asset %1 is 3D but its MXF is marked as 2D.", note.file()->filename());
        case VerificationNote::Code::INVALID_STANDARD:
                return "This DCP does not use the SMPTE standard.";
        case VerificationNote::Code::INVALID_LANGUAGE:
@@ -1434,6 +1525,8 @@ dcp::note_to_string (VerificationNote note)
                return "At least one subtitle lasts less than 15 frames.";
        case VerificationNote::Code::INVALID_SUBTITLE_SPACING:
                return "At least one pair of subtitles is separated by less than 2 frames.";
+       case VerificationNote::Code::SUBTITLE_OVERLAPS_REEL_BOUNDARY:
+               return "At least one subtitle extends outside of its reel.";
        case VerificationNote::Code::INVALID_SUBTITLE_LINE_COUNT:
                return "There are more than 3 subtitle lines in at least one place in the DCP.";
        case VerificationNote::Code::NEARLY_INVALID_SUBTITLE_LINE_LENGTH:
@@ -1520,6 +1613,19 @@ dcp::note_to_string (VerificationNote note)
                return String::compose("The JPEG2000 codestream has %1 tile parts in a 4K image instead of 6.", note.note().get());
        case VerificationNote::Code::MISSING_JPEG200_TLM_MARKER:
                return "No TLM marker was found in a JPEG2000 codestream.";
+       case VerificationNote::Code::MISMATCHED_TIMED_TEXT_RESOURCE_ID:
+               return "The Resource ID in a timed text MXF did not match the ID of the contained XML.";
+       case VerificationNote::Code::INCORRECT_TIMED_TEXT_ASSET_ID:
+               return "The Asset ID in a timed text MXF is the same as the Resource ID or that of the contained XML.";
+       case VerificationNote::Code::MISMATCHED_TIMED_TEXT_DURATION:
+       {
+               vector<string> parts;
+               boost::split (parts, note.note().get(), boost::is_any_of(" "));
+               DCP_ASSERT (parts.size() == 2);
+               return String::compose("The reel duration of some timed text (%1) is not the same as the ContainerDuration of its MXF (%2).", parts[0], parts[1]);
+       }
+       case VerificationNote::Code::MISSED_CHECK_OF_ENCRYPTED:
+               return "Some aspect of this DCP could not be checked because it is encrypted.";
        }
 
        return "";
@@ -1533,6 +1639,29 @@ dcp::operator== (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
 }
 
 
+bool
+dcp::operator< (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
+{
+       if (a.type() != b.type()) {
+               return a.type() < b.type();
+       }
+
+       if (a.code() != b.code()) {
+               return a.code() < b.code();
+       }
+
+       if (a.note() != b.note()) {
+               return a.note().get_value_or("") < b.note().get_value_or("");
+       }
+
+       if (a.file() != b.file()) {
+               return a.file().get_value_or("") < b.file().get_value_or("");
+       }
+
+       return a.line().get_value_or(0) < b.line().get_value_or(0);
+}
+
+
 std::ostream&
 dcp::operator<< (std::ostream& s, dcp::VerificationNote const& note)
 {