From 5d3d4fbd355d8d422a4ac17f93d57ab8ef0a22ee Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 26 Dec 2023 00:42:28 +0100 Subject: [PATCH] Report every frame (with index) that has a JPEG2000 codestream error (DoM #2698). --- src/verify.cc | 27 +++++++++++++++------ src/verify.h | 11 +++++++++ src/verify_j2k.cc | 7 ++++-- src/verify_j2k.h | 5 ++-- test/verify_test.cc | 59 ++++++++++++++++++++++++++++++--------------- 5 files changed, 78 insertions(+), 31 deletions(-) diff --git a/src/verify.cc b/src/verify.cc index c5abb857..5901ffde 100644 --- a/src/verify.cc +++ b/src/verify.cc @@ -438,7 +438,7 @@ verify_language_tag (string tag, vector& notes) static void -verify_picture_asset (shared_ptr reel_file_asset, boost::filesystem::path file, vector& notes, function progress) +verify_picture_asset(shared_ptr reel_file_asset, boost::filesystem::path file, int64_t start_frame, vector& notes, function progress) { int biggest_frame = 0; auto asset = dynamic_pointer_cast(reel_file_asset->asset_ref().asset()); @@ -459,7 +459,7 @@ verify_picture_asset (shared_ptr reel_file_asset, boost::fi biggest_frame = max(biggest_frame, frame->size()); if (!mono_asset->encrypted() || mono_asset->key()) { vector 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 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 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 dcp, shared_ptr reel_asset, + int64_t start_frame, function)> stage, function 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 dcp, shared_ptr cpl, shared_ptr reel, + int64_t start_frame, optional main_picture_active_area, function)> 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()); 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 f57c8e17..7bfe4217 100644 --- 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 @@ -594,6 +596,15 @@ public: return data(Data::OTHER_ID); } + VerificationNote& set_frame_rate(int frame_rate) { + _data[Data::FRAME_RATE] = frame_rate; + return *this; + } + + boost::optional frame_rate() const { + return data(Data::FRAME_RATE); + } + private: Type _type; Code _code; diff --git a/src/verify_j2k.cc b/src/verify_j2k.cc index b9158849..47ee151c 100644 --- a/src/verify_j2k.cc +++ b/src/verify_j2k.cc @@ -65,7 +65,7 @@ public: void -dcp::verify_j2k(shared_ptr j2k, int frame_index, int frame_rate, vector& notes) +dcp::verify_j2k(shared_ptr j2k, int start_index, int frame_index, int frame_rate, vector& 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 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 ac69155c..58c8f4b7 100644 --- 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 data, int frame_index, int frame_rate, std::vector& notes); +void verify_j2k(std::shared_ptr data, int start_index, int frame_index, int frame_rate, std::vector& notes); } diff --git a/test/verify_test.cc b/test/verify_test.cc index 1127c2bf..c382b6d1 100644 --- 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 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 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); } -- 2.30.2