Report every frame (with index) that has a JPEG2000 codestream error (DoM #2698).

diff --git a/src/verify.cc b/src/verify.cc
index c5abb85743b054d3567c1a4a1969f329d3385e24..5901ffde638b510b877e20f75676cc903c4c5719 100644 (file)
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -438,7 +438,7 @@ verify_language_tag (string tag, vector<VerificationNote>& notes)
 
 
 static void
-verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, boost::filesystem::path file, vector<VerificationNote>& notes, function<void (float)> progress)
+verify_picture_asset(shared_ptr<const ReelFileAsset> reel_file_asset, boost::filesystem::path file, int64_t start_frame, vector<VerificationNote>& notes, function<void (float)> progress)
 {
        int biggest_frame = 0;
        auto asset = dynamic_pointer_cast<PictureAsset>(reel_file_asset->asset_ref().asset());
@@ -459,7 +459,7 @@ verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, boost::fi
                        biggest_frame = max(biggest_frame, frame->size());
                        if (!mono_asset->encrypted() || mono_asset->key()) {
                                vector<VerificationNote> j2k_notes;
-                               verify_j2k(frame, i, mono_asset->frame_rate().numerator, j2k_notes);
+                               verify_j2k(frame, start_frame, i, mono_asset->frame_rate().numerator, j2k_notes);
                                check_and_add (j2k_notes);
                        }
                        progress (float(i) / duration);
@@ -471,8 +471,8 @@ verify_picture_asset (shared_ptr<const ReelFileAsset> reel_file_asset, boost::fi
                        biggest_frame = max(biggest_frame, max(frame->left()->size(), frame->right()->size()));
                        if (!stereo_asset->encrypted() || stereo_asset->key()) {
                                vector<VerificationNote> j2k_notes;
-                               verify_j2k(frame->left(), i, stereo_asset->frame_rate().numerator, j2k_notes);
-                               verify_j2k(frame->right(), i, stereo_asset->frame_rate().numerator, j2k_notes);
+                               verify_j2k(frame->left(), start_frame, i, stereo_asset->frame_rate().numerator, j2k_notes);
+                               verify_j2k(frame->right(), start_frame, i, stereo_asset->frame_rate().numerator, j2k_notes);
                                check_and_add (j2k_notes);
                        }
                        progress (float(i) / duration);
@@ -498,6 +498,7 @@ static void
 verify_main_picture_asset (
        shared_ptr<const DCP> dcp,
        shared_ptr<const ReelPictureAsset> reel_asset,
+       int64_t start_frame,
        function<void (string, optional<boost::filesystem::path>)> stage,
        function<void (float)> progress,
        VerificationOptions options,
@@ -527,7 +528,7 @@ verify_main_picture_asset (
        }
 
        stage ("Checking picture frame sizes", asset->file());
-       verify_picture_asset (reel_asset, file, notes, progress);
+       verify_picture_asset(reel_asset, file, start_frame, notes, progress);
 
        /* Only flat/scope allowed by Bv2.1 */
        if (
@@ -1384,6 +1385,7 @@ verify_reel(
        shared_ptr<const DCP> dcp,
        shared_ptr<const CPL> cpl,
        shared_ptr<const Reel> reel,
+       int64_t start_frame,
        optional<dcp::Size> main_picture_active_area,
        function<void (string, optional<boost::filesystem::path>)> stage,
        boost::filesystem::path xsd_dtd_directory,
@@ -1442,7 +1444,7 @@ verify_reel(
                }
                /* Check asset */
                if (reel->main_picture()->asset_ref().resolved()) {
-                       verify_main_picture_asset(dcp, reel->main_picture(), stage, progress, options, notes);
+                       verify_main_picture_asset(dcp, reel->main_picture(), start_frame, stage, progress, options, notes);
                        auto const asset_size = reel->main_picture()->asset()->size();
                        if (main_picture_active_area) {
                                if (main_picture_active_area->width > asset_size.width) {
@@ -1630,12 +1632,14 @@ verify_cpl(
                        });
        }
 
+       int64_t frame = 0;
        for (auto reel: cpl->reels()) {
                stage("Checking reel", optional<boost::filesystem::path>());
                verify_reel(
                        dcp,
                        cpl,
                        reel,
+                       frame,
                        main_picture_active_area,
                        stage,
                        xsd_dtd_directory,
@@ -1649,6 +1653,7 @@ verify_cpl(
                        &fewest_closed_captions,
                        &markers_seen
                        );
+               frame += reel->duration();
        }
 
        verify_text_details(dcp->standard().get_value_or(dcp::Standard::SMPTE), cpl->reels(), notes);
@@ -2034,7 +2039,12 @@ dcp::note_to_string (VerificationNote note)
        case VerificationNote::Code::PARTIALLY_ENCRYPTED:
                return "Some assets are encrypted but some are not.";
        case VerificationNote::Code::INVALID_JPEG2000_CODESTREAM:
-               return String::compose("The JPEG2000 codestream for at least one frame is invalid (%1).", note.note().get());
+               return String::compose(
+                       "Frame %1 (timecode %2) has an invalid JPEG2000 codestream (%2).",
+                       note.frame().get(),
+                       dcp::Time(note.frame().get(), note.frame_rate().get(), note.frame_rate().get()).as_string(dcp::Standard::SMPTE),
+                       note.note().get()
+                       );
        case VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_2K:
                return String::compose("The JPEG2000 codestream uses %1 guard bits in a 2K image instead of 1.", note.note().get());
        case VerificationNote::Code::INVALID_JPEG2000_GUARD_BITS_FOR_4K:
@@ -2133,7 +2143,8 @@ dcp::operator== (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
                a.component() == b.component() &&
                a.size() == b.size() &&
                a.id() == b.id() &&
-               a.other_id() == b.other_id();
+               a.other_id() == b.other_id() &&
+               a.frame_rate() == b.frame_rate();
 }
 
 

diff --git a/src/verify.h b/src/verify.h
index f57c8e17d3b259377ca8cfe102728eeedcf3ab05..7bfe4217c5f934cc77ac8725003363ea0727bcd4 100644 (file)
--- a/src/verify.h
+++ b/src/verify.h
@@ -331,6 +331,7 @@ public:
                /** Some, but not all content, is encrypted */
                PARTIALLY_ENCRYPTED,
                /** General error during JPEG2000 codestream verification
+                *  frame contains the frame index (counted from 0)
                 *  note contains details
                 */
                INVALID_JPEG2000_CODESTREAM,
@@ -524,6 +525,7 @@ private:
                SIZE,
                ID,
                OTHER_ID,
+               FRAME_RATE
        };
 
        template <class T>
@@ -594,6 +596,15 @@ public:
                return data<std::string>(Data::OTHER_ID);
        }
 
+       VerificationNote& set_frame_rate(int frame_rate) {
+               _data[Data::FRAME_RATE] = frame_rate;
+               return *this;
+       }
+
+       boost::optional<int> frame_rate() const {
+               return data<int>(Data::FRAME_RATE);
+       }
+
 private:
        Type _type;
        Code _code;

diff --git a/src/verify_j2k.cc b/src/verify_j2k.cc
index b91588491e1e08242b2a748407fbab0ee4337a8d..47ee151cfe575987b715bf5c8ab0ae319b93e2ca 100644 (file)
--- a/src/verify_j2k.cc
+++ b/src/verify_j2k.cc
@@ -65,7 +65,7 @@ public:
 
 
 void
-dcp::verify_j2k(shared_ptr<const Data> j2k, int frame_index, int frame_rate, vector<VerificationNote>& notes)
+dcp::verify_j2k(shared_ptr<const Data> j2k, int start_index, int frame_index, int frame_rate, vector<VerificationNote>& notes)
 {
        /* See ITU-T T800 (visible on https://github.com/Ymagis/ClairMeta/issues/130) */
        unsigned int const max_tile_part_size = std::floor(200e6 / (8 * frame_rate));
@@ -357,7 +357,10 @@ dcp::verify_j2k(shared_ptr<const Data> j2k, int frame_index, int frame_rate, vec
        }
        catch (InvalidCodestream const& e)
        {
-               notes.push_back ({VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string(e.what()) });
+               VerificationNote note({VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string(e.what())});
+               note.set_frame(start_index + frame_index);
+               note.set_frame_rate(frame_rate);
+               notes.push_back(note);
        }
 }
 

diff --git a/src/verify_j2k.h b/src/verify_j2k.h
index ac69155c61161002a53def8c68db2c71b95bd8d8..58c8f4b7efdfb03293c86e298033bcfc31155a1d 100644 (file)
--- a/src/verify_j2k.h
+++ b/src/verify_j2k.h
@@ -51,11 +51,12 @@ namespace dcp {
 class Data;
 
 
-/** @param frame_index Video frame index, so that notes can say which frame contains the problem.
+/** @param start_index Frame index within the DCP where this frame's reel starts.
+ *  @param frame_index Video frame index within the reel, so that notes can say which frame contains the problem.
  *  @param frame_rate Video frame rate (in frames per second) to calculate how big the tile parts
  *  can be.
  */
-void verify_j2k(std::shared_ptr<const Data> data, int frame_index, int frame_rate, std::vector<VerificationNote>& notes);
+void verify_j2k(std::shared_ptr<const Data> data, int start_index, int frame_index, int frame_rate, std::vector<VerificationNote>& notes);
 
 
 }

diff --git a/test/verify_test.cc b/test/verify_test.cc
index 1127c2bf057d3ccc90625ae155c133c04a962cfa..c382b6d1a699041a92aa499e75de157e043f6d10 100644 (file)
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -635,14 +635,24 @@ BOOST_AUTO_TEST_CASE (verify_invalid_picture_frame_size_in_bytes)
        prepare_directory (dir);
        auto cpl = dcp_from_frame (oversized_frame, dir);
 
-       check_verify_result (
-               { dir },
-               {},
-               {
-                       { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
-                       { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") },
-                       { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
-               });
+       vector<dcp::VerificationNote> expected;
+       for (auto i = 0; i < 24; ++i) {
+               expected.push_back(
+                       dcp::VerificationNote(
+                               dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte")
+                               ).set_frame(i).set_frame_rate(24)
+                       );
+       }
+
+       expected.push_back(
+               { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") }
+       );
+
+       expected.push_back(
+               { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+       );
+
+       check_verify_result({ dir }, {}, expected);
 }
 
 
@@ -664,14 +674,25 @@ BOOST_AUTO_TEST_CASE (verify_nearly_invalid_picture_frame_size_in_bytes)
        prepare_directory (dir);
        auto cpl = dcp_from_frame (oversized_frame, dir);
 
-       check_verify_result (
-               { dir },
-               {},
-               {
-                       { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte") },
-                       { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") },
-                       { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
-               });
+       vector<dcp::VerificationNote> expected;
+
+       for (auto i = 0; i < 24; ++i) {
+               expected.push_back(
+                       dcp::VerificationNote(
+                               dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_JPEG2000_CODESTREAM, string("missing marker start byte")
+                               ).set_frame(i).set_frame_rate(24)
+                       );
+       }
+
+       expected.push_back(
+               { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") }
+       );
+
+       expected.push_back(
+               { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+       );
+
+       check_verify_result ({ dir }, {}, expected);
 }
 
 
@@ -3097,7 +3118,7 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_2k)
        dcp::MonoPictureAsset picture (find_file(private_test / "data" / "JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV", "j2c.mxf"));
        auto reader = picture.start_read ();
        auto frame = reader->get_frame (0);
-       verify_j2k(frame, 0, 24, notes);
+       verify_j2k(frame, 0, 0, 24, notes);
        BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }
 
@@ -3108,7 +3129,7 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_4k)
        dcp::MonoPictureAsset picture (find_file(private_test / "data" / "sul", "TLR"));
        auto reader = picture.start_read ();
        auto frame = reader->get_frame (0);
-       verify_j2k(frame, 0, 24, notes);
+       verify_j2k(frame, 0, 0, 24, notes);
        BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }
 
@@ -3123,7 +3144,7 @@ BOOST_AUTO_TEST_CASE (verify_jpeg2000_codestream_libdcp)
        dcp::MonoPictureAsset picture (find_file(dir, "video"));
        auto reader = picture.start_read ();
        auto frame = reader->get_frame (0);
-       verify_j2k(frame, 0, 24, notes);
+       verify_j2k(frame, 0, 0, 24, notes);
        BOOST_REQUIRE_EQUAL (notes.size(), 0U);
 }