Move verify API away from strings towards error codes.

diff --git a/src/verify.cc b/src/verify.cc
index 2288200b720d2e742b5d24bc274407bda22bdded..4a6568743597f2fc49452c57fcefd3ef2c850da4 100644 (file)
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2018 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2018-2019 Carl Hetherington <cth@carlh.net>
 
     This file is part of libdcp.
 
@@ -38,6 +38,7 @@
 #include "reel_picture_asset.h"
 #include "reel_sound_asset.h"
 #include "exceptions.h"
+#include "compose.hpp"
 #include <boost/foreach.hpp>
 #include <list>
 #include <vector>
@@ -108,9 +109,9 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
                try {
                        dcp->read (true, &errors);
                } catch (DCPReadError& e) {
-                       notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, e.what ()));
+                       notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
                } catch (XMLError& e) {
-                       notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, e.what ()));
+                       notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::GENERAL_READ, string(e.what())));
                }
 
                BOOST_FOREACH (shared_ptr<CPL> cpl, dcp->cpls()) {
@@ -120,7 +121,7 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
                        BOOST_FOREACH (shared_ptr<PKL> i, dcp->pkls()) {
                                optional<string> h = i->hash(cpl->id());
                                if (h && make_digest(Data(*cpl->file())) != *h) {
-                                       notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, "CPL hash is incorrect."));
+                                       notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::CPL_HASH_INCORRECT));
                                }
                        }
 
@@ -136,17 +137,21 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
                                             frame_rate.numerator != 48 &&
                                             frame_rate.numerator != 50 &&
                                             frame_rate.numerator != 60)) {
-                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "Invalid frame rate for picture"));
+                                               notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::INVALID_PICTURE_FRAME_RATE));
                                        }
                                        /* Check asset */
                                        stage ("Checking picture asset hash", reel->main_picture()->asset()->file());
                                        Result const r = verify_asset (dcp, reel->main_picture(), progress);
                                        switch (r) {
                                        case RESULT_BAD:
-                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "Picture asset hash is incorrect."));
+                                               notes.push_back (
+                                                       VerificationNote(
+                                                               VerificationNote::VERIFY_ERROR, VerificationNote::PICTURE_HASH_INCORRECT, *reel->main_picture()->asset()->file()
+                                                               )
+                                                       );
                                                break;
                                        case RESULT_CPL_PKL_DIFFER:
-                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "PKL and CPL hashes differ for picture asset."));
+                                               notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE));
                                                break;
                                        default:
                                                break;
@@ -157,10 +162,14 @@ dcp::verify (vector<boost::filesystem::path> directories, function<void (string,
                                        Result const r = verify_asset (dcp, reel->main_sound(), progress);
                                        switch (r) {
                                        case RESULT_BAD:
-                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "Sound asset hash is incorrect."));
+                                               notes.push_back (
+                                                       VerificationNote(
+                                                               VerificationNote::VERIFY_ERROR, VerificationNote::SOUND_HASH_INCORRECT, *reel->main_sound()->asset()->file()
+                                                               )
+                                                       );
                                                break;
                                        case RESULT_CPL_PKL_DIFFER:
-                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, "PKL and CPL hashes differ for sound asset."));
+                                               notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE));
                                                break;
                                        default:
                                                break;

diff --git a/src/verify.h b/src/verify.h
index f5b21763eb4de1310e5ee066c517d4893a82d207..4b967d1d590de1c43795aea02924a59fbf6d6f61 100644 (file)
--- a/src/verify.h
+++ b/src/verify.h
@@ -51,26 +51,64 @@ public:
        */
        enum Type {
                VERIFY_ERROR,
-               VERIFY_WARNING,
-               VERIFY_NOTE
+               VERIFY_WARNING
        };
 
-       VerificationNote (Type type, std::string note)
+       enum Code {
+               /** An error when reading the DCP.  note contains (probably technical) details. */
+               GENERAL_READ,
+               /** The hash of the CPL in the PKL does not agree with the CPL file */
+               CPL_HASH_INCORRECT,
+               /** Frame rate given in a reel for the main picture is not 24, 25, 30, 48, 50 or 60 */
+               INVALID_PICTURE_FRAME_RATE,
+               /** The hash of a main picture asset does not agree with the PKL file.  file contains the picture asset filename. */
+               PICTURE_HASH_INCORRECT,
+               /** The hash of a main picture is different in the CPL and PKL */
+               PKL_CPL_PICTURE_HASHES_DISAGREE,
+               /** The hash of a main sound asset does not agree with the PKL file.  file contains the sound asset filename. */
+               SOUND_HASH_INCORRECT,
+               /** The hash of a main sound is different in the CPL and PKL */
+               PKL_CPL_SOUND_HASHES_DISAGREE,
+       };
+
+       VerificationNote (Type type, Code code)
+               : _type (type)
+               , _code (code)
+       {}
+
+       VerificationNote (Type type, Code code, std::string note)
                : _type (type)
+               , _code (code)
                , _note (note)
        {}
 
+       VerificationNote (Type type, Code code, boost::filesystem::path file)
+               : _type (type)
+               , _code (code)
+               , _file (file)
+       {}
+
        Type type () const {
                return _type;
        }
 
-       std::string note () const {
+       Code code () const {
+               return _code;
+       }
+
+       boost::optional<std::string> note () const {
                return _note;
        }
 
+       boost::optional<boost::filesystem::path> file () const {
+               return _file;
+       }
+
 private:
        Type _type;
-       std::string _note;
+       Code _code;
+       boost::optional<std::string> _note;
+       boost::optional<boost::filesystem::path> _file;
 };
 
 std::list<VerificationNote> verify (

diff --git a/test/verify_test.cc b/test/verify_test.cc
index a7e703a21447c0de11df4fe635af16aeb4e6cd07..a4ed162d562bfa5c0d9a3ceeb82b40051a1dd8a9 100644 (file)
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -130,9 +130,9 @@ BOOST_AUTO_TEST_CASE (verify_test2)
 
        BOOST_REQUIRE_EQUAL (notes.size(), 2);
        BOOST_CHECK_EQUAL (notes.front().type(), dcp::VerificationNote::VERIFY_ERROR);
-       BOOST_CHECK_EQUAL (notes.front().note(), "Picture asset hash is incorrect.");
+       BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::PICTURE_HASH_INCORRECT);
        BOOST_CHECK_EQUAL (notes.back().type(), dcp::VerificationNote::VERIFY_ERROR);
-       BOOST_CHECK_EQUAL (notes.back().note(), "Sound asset hash is incorrect.");
+       BOOST_CHECK_EQUAL (notes.back().code(), dcp::VerificationNote::SOUND_HASH_INCORRECT);
 }
 
 /* Corrupt the hashes in the PKL and check that the disagreement between CPL and PKL is spotted */
@@ -153,13 +153,13 @@ BOOST_AUTO_TEST_CASE (verify_test3)
        BOOST_REQUIRE_EQUAL (notes.size(), 3);
        list<dcp::VerificationNote>::const_iterator i = notes.begin();
        BOOST_CHECK_EQUAL (i->type(), dcp::VerificationNote::VERIFY_ERROR);
-       BOOST_CHECK_EQUAL (i->note(), "CPL hash is incorrect.");
+       BOOST_CHECK_EQUAL (i->code(), dcp::VerificationNote::CPL_HASH_INCORRECT);
        ++i;
        BOOST_CHECK_EQUAL (i->type(), dcp::VerificationNote::VERIFY_ERROR);
-       BOOST_CHECK_EQUAL (i->note(), "PKL and CPL hashes differ for picture asset.");
+       BOOST_CHECK_EQUAL (i->code(), dcp::VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE);
        ++i;
        BOOST_CHECK_EQUAL (i->type(), dcp::VerificationNote::VERIFY_ERROR);
-       BOOST_CHECK_EQUAL (i->note(), "PKL and CPL hashes differ for sound asset.");
+       BOOST_CHECK_EQUAL (i->code(), dcp::VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE);
        ++i;
 }
 
@@ -179,7 +179,8 @@ BOOST_AUTO_TEST_CASE (verify_test4)
        list<dcp::VerificationNote> notes = dcp::verify (directories, &stage, &progress);
 
        BOOST_REQUIRE_EQUAL (notes.size(), 1);
-       BOOST_CHECK_EQUAL (notes.front().note(), "Bad content kind 'xfeature'");
+       BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::GENERAL_READ);
+       BOOST_CHECK_EQUAL (*notes.front().note(), "Bad content kind 'xfeature'");
 }
 
 /* FrameRate */
@@ -198,6 +199,6 @@ BOOST_AUTO_TEST_CASE (verify_test5)
        list<dcp::VerificationNote> notes = dcp::verify (directories, &stage, &progress);
 
        BOOST_REQUIRE_EQUAL (notes.size(), 2);
-       BOOST_CHECK_EQUAL (notes.front().note(), "CPL hash is incorrect.");
-       BOOST_CHECK_EQUAL (notes.back().note(), "Invalid frame rate for picture");
+       BOOST_CHECK_EQUAL (notes.front().code(), dcp::VerificationNote::CPL_HASH_INCORRECT);
+       BOOST_CHECK_EQUAL (notes.back().code(), dcp::VerificationNote::INVALID_PICTURE_FRAME_RATE);
 }

diff --git a/tools/dcpverify.cc b/tools/dcpverify.cc
index d246dc659821879a4c50da76df2bd2ef54b226cd..675b75223f1a5ca7625d9dd82b5550c2fd04c50b 100644 (file)
--- a/tools/dcpverify.cc
+++ b/tools/dcpverify.cc
@@ -32,6 +32,7 @@
 */
 
 #include "verify.h"
+#include "compose.hpp"
 #include <boost/bind.hpp>
 #include <boost/optional.hpp>
 #include <boost/filesystem.hpp>
@@ -72,6 +73,29 @@ progress ()
 
 }
 
+std::string
+note_to_string (dcp::VerificationNote note)
+{
+       switch (note.code()) {
+       case dcp::VerificationNote::GENERAL_READ:
+               return *note.note();
+       case dcp::VerificationNote::CPL_HASH_INCORRECT:
+               return "The hash of the CPL in the PKL does not agree with the CPL file";
+       case dcp::VerificationNote::INVALID_PICTURE_FRAME_RATE:
+               return "The picture in a reel has an invalid frame rate";
+       case dcp::VerificationNote::PICTURE_HASH_INCORRECT:
+               return dcp::String::compose("The hash of the picture asset %1 does not agree with the PKL file", note.file()->filename());
+       case dcp::VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE:
+               return "The PKL and CPL hashes disagree for a picture asset.";
+       case dcp::VerificationNote::SOUND_HASH_INCORRECT:
+               return dcp::String::compose("The hash of the sound asset %1 does not agree with the PKL file", note.file()->filename());
+       case dcp::VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE:
+               return "The PKL and CPL hashes disagree for a sound asset.";
+       }
+
+       return "";
+}
+
 int
 main (int argc, char* argv[])
 {
@@ -117,14 +141,11 @@ main (int argc, char* argv[])
        BOOST_FOREACH (dcp::VerificationNote i, notes) {
                switch (i.type()) {
                case dcp::VerificationNote::VERIFY_ERROR:
-                       cout << "Error: " << i.note() << "\n";
+                       cout << "Error: " << note_to_string(i) << "\n";
                        failed = true;
                        break;
                case dcp::VerificationNote::VERIFY_WARNING:
-                       cout << "Warning: " << i.note() << "\n";
-                       break;
-               case dcp::VerificationNote::VERIFY_NOTE:
-                       cout << "Note: " << i.note() << "\n";
+                       cout << "Warning: " << note_to_string(i) << "\n";
                        break;
                }
        }