Fix equals() with image subtitles to not compare unique IDs.

Also add an option to save subtitle images to PNGs when they differ.

diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index c6d2e7908f81113e52b736723d5048709f0a4c20..96e5cba5e5797de45812693ad37b133c7d7ae611 100644 (file)
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -452,7 +452,7 @@ SubtitleAsset::equals (shared_ptr<const Asset> other_asset, EqualityOptions opti
        }
 
        if (_subtitles.size() != other->_subtitles.size()) {
        }
 
        if (_subtitles.size() != other->_subtitles.size()) {

-               note (DCP_ERROR, "subtitles differ");
+               note (DCP_ERROR, String::compose("different number of subtitles: %1 vs %2", _subtitles.size(), other->_subtitles.size()));

                return false;
        }
 
                return false;
        }
 


@@ -466,17 +466,16 @@ SubtitleAsset::equals (shared_ptr<const Asset> other_asset, EqualityOptions opti
                shared_ptr<SubtitleImage> image_j = dynamic_pointer_cast<SubtitleImage> (*j);
 
                if ((string_i && !string_j) || (image_i && !image_j)) {
                shared_ptr<SubtitleImage> image_j = dynamic_pointer_cast<SubtitleImage> (*j);
 
                if ((string_i && !string_j) || (image_i && !image_j)) {

-                       note (DCP_ERROR, "subtitles differ");
+                       note (DCP_ERROR, "subtitles differ: string vs. image");

                        return false;
                }
 
                if (string_i && *string_i != *string_j) {
                        return false;
                }
 
                if (string_i && *string_i != *string_j) {

-                       note (DCP_ERROR, "subtitles differ");
+                       note (DCP_ERROR, String::compose("subtitles differ in text or metadata: %1 vs %2", string_i->text(), string_j->text()));

                        return false;
                }
 
                        return false;
                }
 

-               if (image_i && *image_i != *image_j) {
-                       note (DCP_ERROR, "subtitles differ");
+               if (image_i && !image_i->equals(image_j, options, note)) {

                        return false;
                }
 
                        return false;
                }
 

diff --git a/src/subtitle_image.cc b/src/subtitle_image.cc
index d8215850784e6abb1c1255165e7fcd3bdf6b2dc5..e280e3921274351737725c4e25657819ca2a7fc7 100644 (file)
--- a/src/subtitle_image.cc
+++ b/src/subtitle_image.cc
@@ -1,5 +1,5 @@
 /*
 /*

-    Copyright (C) 2018 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2018-2020 Carl Hetherington <cth@carlh.net>

 
     This file is part of libdcp.
 
 
     This file is part of libdcp.
 


@@ -36,6 +36,7 @@
 
 using std::ostream;
 using std::string;
 
 using std::ostream;
 using std::string;

+using boost::shared_ptr;

 using namespace dcp;
 
 SubtitleImage::SubtitleImage (
 using namespace dcp;
 
 SubtitleImage::SubtitleImage (


@@ -112,6 +113,71 @@ dcp::operator!= (SubtitleImage const & a, SubtitleImage const & b)
        return !(a == b);
 }
 
        return !(a == b);
 }
 

+bool
+SubtitleImage::equals (shared_ptr<SubtitleImage> other, EqualityOptions options, NoteHandler note)
+{
+       if (png_image() != other->png_image()) {
+               note (DCP_ERROR, "subtitle image PNG data differs");
+               if (options.export_differing_subtitles) {
+                       string const base = "dcpdiff_subtitle_";
+                       if (boost::filesystem::exists(base + "A.png")) {
+                               note (DCP_ERROR, "could not export subtitle as " + base + "A.png already exists");
+                       } else {
+                               png_image().write(base + "A.png");
+                       }
+                       if (boost::filesystem::exists(base + "B.png")) {
+                               note (DCP_ERROR, "could not export subtitle as " + base + "B.png already exists");
+                       } else {
+                               other->png_image().write(base + "B.png");
+                       }
+                       options.export_differing_subtitles = false;
+               }
+               return false;
+       }
+
+       if (in() != other->in()) {
+               note (DCP_ERROR, "subtitle in times differ");
+               return false;
+       }
+
+       if (out() != other->out()) {
+               note (DCP_ERROR, "subtitle out times differ");
+               return false;
+       }
+
+       if (h_position() != other->h_position()) {
+               note (DCP_ERROR, "subtitle horizontal positions differ");
+               return false;
+       }
+
+       if (h_align() != other->h_align()) {
+               note (DCP_ERROR, "subtitle horizontal alignments differ");
+               return false;
+       }
+
+       if (v_position() != other->v_position()) {
+               note (DCP_ERROR, "subtitle vertical positions differ");
+               return false;
+       }
+
+       if (v_align() != other->v_align()) {
+               note (DCP_ERROR, "subtitle vertical alignments differ");
+               return false;
+       }
+
+       if (fade_up_time() != other->fade_up_time()) {
+               note (DCP_ERROR, "subtitle fade-up times differ");
+               return false;
+       }
+
+       if (fade_down_time() != other->fade_down_time()) {
+               note (DCP_ERROR, "subtitle fade-down times differ");
+               return false;
+       }
+
+       return true;
+}
+

 ostream&
 dcp::operator<< (ostream& s, SubtitleImage const & sub)
 {
 ostream&
 dcp::operator<< (ostream& s, SubtitleImage const & sub)
 {


@@ -122,3 +188,4 @@ dcp::operator<< (ostream& s, SubtitleImage const & sub)
 
        return s;
 }
 
        return s;
 }

+

diff --git a/src/subtitle_image.h b/src/subtitle_image.h
index 76c347653c1626c602c273d79f5e90deb0988ede..e397fb2b1e4127f4827e37702d2ac84fc86bce7a 100644 (file)
--- a/src/subtitle_image.h
+++ b/src/subtitle_image.h
@@ -99,6 +99,8 @@ public:
                return _file;
        }
 
                return _file;
        }
 

+       bool equals (boost::shared_ptr<dcp::SubtitleImage> other, EqualityOptions options, NoteHandler note);
+

 private:
        ArrayData _png_image;
        std::string _id;
 private:
        ArrayData _png_image;
        std::string _id;

diff --git a/src/types.h b/src/types.h
index 684145dd25fc13a681a7046fee5946f455182720..1102f16b56bbaea6a30c078ed5619e117e2e4b5d 100644 (file)
--- a/src/types.h
+++ b/src/types.h
@@ -215,6 +215,8 @@ extern std::ostream& operator<< (std::ostream& s, Fraction const & f);
  *
  *  When comparing things, we want to be able to ignore some differences;
  *  this class expresses those differences.
  *
  *  When comparing things, we want to be able to ignore some differences;
  *  this class expresses those differences.

+ *
+ *  It also contains some settings for how the comparison should be done.

  */
 struct EqualityOptions
 {
  */
 struct EqualityOptions
 {


@@ -229,6 +231,7 @@ struct EqualityOptions
                , issue_dates_can_differ (false)
                , load_font_nodes_can_differ (false)
                , keep_going (false)
                , issue_dates_can_differ (false)
                , load_font_nodes_can_differ (false)
                , keep_going (false)

+               , export_differing_subtitles (false)

        {}
 
        /** The maximum allowable mean difference in pixel value between two images */
        {}
 
        /** The maximum allowable mean difference in pixel value between two images */


@@ -247,6 +250,8 @@ struct EqualityOptions
        bool issue_dates_can_differ;
        bool load_font_nodes_can_differ;
        bool keep_going;
        bool issue_dates_can_differ;
        bool load_font_nodes_can_differ;
        bool keep_going;

+       /** true to save the first pair of differeng image subtitles to the current working directory */
+       bool export_differing_subtitles;

 };
 
 /* I've been unable to make mingw happy with ERROR as a symbol, so
 };
 
 /* I've been unable to make mingw happy with ERROR as a symbol, so

diff --git a/tools/dcpdiff.cc b/tools/dcpdiff.cc
index 3940c0d49ae1af852034133b3aab550492ea9af7..2bda4347ecf5816d700d7a42d9e14ede8d04ba02 100644 (file)
--- a/tools/dcpdiff.cc
+++ b/tools/dcpdiff.cc
@@ -61,17 +61,18 @@ static void
 help (string n)
 {
        cerr << "Syntax: " << n << " [OPTION] <DCP> <DCP>\n"
 help (string n)
 {
        cerr << "Syntax: " << n << " [OPTION] <DCP> <DCP>\n"

-            << "  -V, --version                show libdcp version\n"
-            << "  -h, --help                   show this help\n"
-            << "  -v, --verbose                be verbose\n"
-            << "      --cpl-annotation-texts   allow differing CPL annotation texts\n"
-            << "      --reel-annotation-texts  allow differing reel annotation texts\n"
-            << "  -a, --annotation-texts       allow different CPL and reel annotation texts\n"
-            << "  -d, --issue-dates            allow different issue dates\n"
-            << "  -m, --mean-pixel             maximum allowed mean pixel error (default 5)\n"
-            << "  -s, --std-dev-pixel          maximum allowed standard deviation of pixel error (default 5)\n"
-            << "      --key                    hexadecimal key to use to decrypt MXFs\n"
-            << "      --ignore-missing-assets  ignore missing asset files\n"
+            << "  -V, --version                     show libdcp version\n"
+            << "  -h, --help                        show this help\n"
+            << "  -v, --verbose                     be verbose\n"
+            << "      --cpl-annotation-texts        allow differing CPL annotation texts\n"
+            << "      --reel-annotation-texts       allow differing reel annotation texts\n"
+            << "  -a, --annotation-texts            allow different CPL and reel annotation texts\n"
+            << "  -d, --issue-dates                 allow different issue dates\n"
+            << "  -m, --mean-pixel                  maximum allowed mean pixel error (default 5)\n"
+            << "  -s, --std-dev-pixel               maximum allowed standard deviation of pixel error (default 5)\n"
+            << "      --key                         hexadecimal key to use to decrypt MXFs\n"
+            << "      --ignore-missing-assets       ignore missing asset files\n"
+            << "      --export-differing-subtitles  export the first pair of differing image subtitles to the current working directory\n"

             << "\n"
             << "The <DCP>s are the DCP directories to compare.\n"
             << "Comparison is of metadata and content, ignoring timestamps\n"
             << "\n"
             << "The <DCP>s are the DCP directories to compare.\n"
             << "Comparison is of metadata and content, ignoring timestamps\n"


@@ -147,10 +148,11 @@ main (int argc, char* argv[])
                        { "cpl-annotation-texts", no_argument, 0, 'C'},
                        { "key", required_argument, 0, 'D'},
                        { "reel-annotation-texts", no_argument, 0, 'E'},
                        { "cpl-annotation-texts", no_argument, 0, 'C'},
                        { "key", required_argument, 0, 'D'},
                        { "reel-annotation-texts", no_argument, 0, 'E'},

+                       { "export-differing-subtitles", no_argument, 0, 'F' },

                        { 0, 0, 0, 0 }
                };
 
                        { 0, 0, 0, 0 }
                };
 

-               int c = getopt_long (argc, argv, "Vhvm:s:adACD:E", long_options, &option_index);
+               int c = getopt_long (argc, argv, "Vhvm:s:adACD:EF", long_options, &option_index);

 
                if (c == -1) {
                        break;
 
                if (c == -1) {
                        break;


@@ -191,6 +193,9 @@ main (int argc, char* argv[])
                case 'E':
                        options.reel_annotation_texts_can_differ = true;
                        break;
                case 'E':
                        options.reel_annotation_texts_can_differ = true;
                        break;

+               case 'F':
+                       options.export_differing_subtitles = true;
+                       break;

                }
        }
 
                }
        }