Report every frame (with index) that is over (or nearly over) the size limit (DoM...

diff --git a/src/verify.cc b/src/verify.cc
index 5901ffde638b510b877e20f75676cc903c4c5719..f20c046d02ad29e93745c20a54eaddf46989a1bb 100644 (file)
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -440,7 +440,6 @@ verify_language_tag (string tag, vector<VerificationNote>& notes)
 static void
 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());
        auto const duration = asset->intrinsic_duration ();
 
@@ -452,11 +451,30 @@ verify_picture_asset(shared_ptr<const ReelFileAsset> reel_file_asset, boost::fil
                }
        };
 
+       int const max_frame =   rint(250 * 1000000 / (8 * asset->edit_rate().as_float()));
+       int const risky_frame = rint(230 * 1000000 / (8 * asset->edit_rate().as_float()));
+
+       auto check_frame_size = [max_frame, risky_frame, file, start_frame](int index, int size, int frame_rate, vector<VerificationNote>& notes) {
+               if (size > max_frame) {
+                       notes.push_back(
+                               VerificationNote(
+                                       VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+                                       ).set_frame(start_frame + index).set_frame_rate(frame_rate)
+                       );
+               } else if (size > risky_frame) {
+                       notes.push_back(
+                               VerificationNote(
+                                       VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
+                                       ).set_frame(start_frame + index).set_frame_rate(frame_rate)
+                       );
+               }
+       };
+
        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());
+                       check_frame_size(i, frame->size(), mono_asset->frame_rate().numerator, notes);
                        if (!mono_asset->encrypted() || mono_asset->key()) {
                                vector<VerificationNote> j2k_notes;
                                verify_j2k(frame, start_frame, i, mono_asset->frame_rate().numerator, j2k_notes);
@@ -468,7 +486,8 @@ verify_picture_asset(shared_ptr<const ReelFileAsset> reel_file_asset, boost::fil
                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()));
+                       check_frame_size(i, frame->left()->size(), stereo_asset->frame_rate().numerator, notes);
+                       check_frame_size(i, frame->right()->size(), stereo_asset->frame_rate().numerator, notes);
                        if (!stereo_asset->encrypted() || stereo_asset->key()) {
                                vector<VerificationNote> j2k_notes;
                                verify_j2k(frame->left(), start_frame, i, stereo_asset->frame_rate().numerator, j2k_notes);
@@ -479,18 +498,6 @@ verify_picture_asset(shared_ptr<const ReelFileAsset> reel_file_asset, boost::fil
                }
 
        }
-
-       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) {
-               notes.push_back ({
-                       VerificationNote::Type::ERROR, VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
-               });
-       } else if (biggest_frame > risky_frame) {
-               notes.push_back ({
-                       VerificationNote::Type::WARNING, VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, file
-               });
-       }
 }
 
 
@@ -1937,9 +1944,19 @@ dcp::note_to_string (VerificationNote note)
        case VerificationNote::Code::INVALID_DURATION:
                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());
+               return String::compose(
+                       "Frame %1 (timecode %2) in asset %3 has an instantaneous bit rate that is larger than the limit of 250Mbit/s.",
+                       note.frame().get(),
+                       dcp::Time(note.frame().get(), note.frame_rate().get(), note.frame_rate().get()).as_string(dcp::Standard::SMPTE),
+                       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());
+               return String::compose(
+                       "Frame %1 (timecode %2) in asset %3 has an instantaneous bit rate that is close to the limit of 250Mbit/s.",
+                       note.frame().get(),
+                       dcp::Time(note.frame().get(), note.frame_rate().get(), note.frame_rate().get()).as_string(dcp::Standard::SMPTE),
+                       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:
@@ -2187,7 +2204,11 @@ dcp::operator< (dcp::VerificationNote const& a, dcp::VerificationNote const& b)
                return a.id().get_value_or("") < b.id().get_value_or("");
        }
 
-       return a.other_id().get_value_or("") < b.other_id().get_value_or("");
+       if (a.other_id() != b.other_id()) {
+               return a.other_id().get_value_or("") < b.other_id().get_value_or("");
+       }
+
+       return a.frame_rate().get_value_or(0) != b.frame_rate().get_value_or(0);
 }
 
 

diff --git a/test/verify_test.cc b/test/verify_test.cc
index c382b6d1a699041a92aa499e75de157e043f6d10..0e15624d48d836186e906d1bd5cbf132f6716420 100644 (file)
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -644,9 +644,13 @@ BOOST_AUTO_TEST_CASE (verify_invalid_picture_frame_size_in_bytes)
                        );
        }
 
-       expected.push_back(
-               { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") }
-       );
+       for (auto i = 0; i < 24; ++i) {
+               expected.push_back(
+                       dcp::VerificationNote(
+                               dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf")
+                               ).set_frame(i).set_frame_rate(24)
+                       );
+       }
 
        expected.push_back(
                { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
@@ -684,9 +688,13 @@ BOOST_AUTO_TEST_CASE (verify_nearly_invalid_picture_frame_size_in_bytes)
                        );
        }
 
-       expected.push_back(
-               { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf") }
-       );
+       for (auto i = 0; i < 24; ++i) {
+               expected.push_back(
+                       dcp::VerificationNote(
+                               dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::NEARLY_INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(dir / "pic.mxf")
+                               ).set_frame(i).set_frame_rate(24)
+               );
+       }
 
        expected.push_back(
                { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
@@ -3668,9 +3676,13 @@ BOOST_AUTO_TEST_CASE(verify_invalid_tile_part_size)
 
        vector<dcp::VerificationNote> expected;
 
-       expected.push_back(
-               { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(path / "video.mxf") }
-       );
+       for (auto frame = 0; frame < 24; frame++) {
+               expected.push_back(
+                       dcp::VerificationNote(
+                               dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::INVALID_PICTURE_FRAME_SIZE_IN_BYTES, canonical(path / "video.mxf")
+                       ).set_frame(frame).set_frame_rate(24)
+               );
+       }
 
        int component_sizes[] = {
                1321721,