From 7c7e27184c12c2613a389237933ee58723f2d97f Mon Sep 17 00:00:00 2001 From: sz Date: Sun, 20 Oct 2024 22:26:33 -0500 Subject: [PATCH 1/4] For encodes via the cimbar cli, skip over frames that will fail extract Some cimbar frames are bad and will fail extraction due to the data aligning itself to match the anchor pattern. This is a design bug/quirk that doesn't matter *too* much for the on-the-fly "encode data on one screen, decode via camera" use case. We're constantly generating new frames. But for any offline, more "QR-code" like usage -- for example printing -- having a bad image is much more troublesome. The fix/workaround is to check the generated image after we generate, and make sure it extracts. If it doesn't, we skip that frame and try again. This is currently only used the cli (and various test code), due to potential performance concerns. --- src/exe/cimbar/cimbar.cpp | 1 + src/lib/encoder/Encoder.h | 8 +++++ src/lib/encoder/test/CMakeLists.txt | 1 + src/lib/encoder/test/EncoderRoundTripTest.cpp | 2 +- src/lib/encoder/test/EncoderTest.cpp | 12 +++---- src/lib/extractor/Scanner.h | 35 ++++++++++++++++--- 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/exe/cimbar/cimbar.cpp b/src/exe/cimbar/cimbar.cpp index 636998f8..35c00988 100644 --- a/src/exe/cimbar/cimbar.cpp +++ b/src/exe/cimbar/cimbar.cpp @@ -104,6 +104,7 @@ template int encode(const FilenameIterable& infiles, const std::string& outpath, int ecc, int color_bits, int compression_level, bool legacy_mode, bool no_fountain) { Encoder en(ecc, cimbar::Config::symbol_bits(), color_bits); + en.set_encode_id(109); if (legacy_mode) en.set_legacy_mode(); for (const string& f : infiles) diff --git a/src/lib/encoder/Encoder.h b/src/lib/encoder/Encoder.h index 9f16b40e..2115ab2f 100644 --- a/src/lib/encoder/Encoder.h +++ b/src/lib/encoder/Encoder.h @@ -3,6 +3,7 @@ #include "SimpleEncoder.h" #include "cimb_translator/Config.h" +#include "extractor/Scanner.h" #include "serialize/format.h" #include @@ -61,6 +62,13 @@ inline unsigned Encoder::encode_fountain(const std::string& filename, const std: if (!frame) break; + // some % of generated frames (for the current 8x8 impl) + // will produce random patterns that falsely match as + // corner "anchors" and fail to extract. So: + // if frame fails the scan, skip it. + if (!Scanner::will_it_scan(*frame)) + continue; + if (!on_frame(*frame, i)) break; ++i; diff --git a/src/lib/encoder/test/CMakeLists.txt b/src/lib/encoder/test/CMakeLists.txt index 14938610..78e2d2f5 100644 --- a/src/lib/encoder/test/CMakeLists.txt +++ b/src/lib/encoder/test/CMakeLists.txt @@ -27,6 +27,7 @@ add_test(encoder_test encoder_test) target_link_libraries(encoder_test cimb_translator + extractor correct_static wirehair diff --git a/src/lib/encoder/test/EncoderRoundTripTest.cpp b/src/lib/encoder/test/EncoderRoundTripTest.cpp index 0193e06e..98812308 100644 --- a/src/lib/encoder/test/EncoderRoundTripTest.cpp +++ b/src/lib/encoder/test/EncoderRoundTripTest.cpp @@ -31,7 +31,7 @@ TEST_CASE( "EncoderRoundTripTest/testFountain.Pad", "[unit]" ) Encoder enc(30, 4, 2); assertEquals( 1, enc.encode_fountain(inputFile, outPrefix) ); - uint64_t hash = 0xeecc8800efce8c48; + uint64_t hash = 0xefcc8800efce8c48; std::string path = fmt::format("{}_0.png", outPrefix); cv::Mat encodedImg = cv::imread(path); cv::cvtColor(encodedImg, encodedImg, cv::COLOR_BGR2RGB); diff --git a/src/lib/encoder/test/EncoderTest.cpp b/src/lib/encoder/test/EncoderTest.cpp index 60afa677..73917428 100644 --- a/src/lib/encoder/test/EncoderTest.cpp +++ b/src/lib/encoder/test/EncoderTest.cpp @@ -49,7 +49,7 @@ TEST_CASE( "EncoderTest/testFountain.4c", "[unit]" ) enc.set_legacy_mode(); assertEquals( 3, enc.encode_fountain(inputFile, outPrefix, 0) ); - std::vector hashes = {0xbb1cc62b662abfe5, 0xf586f6466a5b194, 0x8c2f0f40e6ecb08b}; + std::vector hashes = {0xbb1cc62b662abfe5, 0xf586f6466a5b194, 0x93a3830d042966e1}; for (unsigned i = 0; i < hashes.size(); ++i) { DYNAMIC_SECTION( "are we correct? : " << i ) @@ -93,7 +93,7 @@ TEST_CASE( "EncoderTest/testFountain.Compress", "[unit]" ) Encoder enc(30, 4, 2); assertEquals( 1, enc.encode_fountain(inputFile, outPrefix) ); - uint64_t hash = 0xb36b65402eec434e; + uint64_t hash = 0xa66a666543280e8e; std::string path = fmt::format("{}_0.png", outPrefix); cv::Mat img = cv::imread(path); assertEquals( hash, image_hash::average_hash(img) ); @@ -131,12 +131,12 @@ TEST_CASE( "EncoderTest/testFountain.Size", "[unit]" ) std::string outPrefix = tempdir.path() / "encoder.fountain"; Encoder enc(30, 4, 2); - assertEquals( 1, enc.encode_fountain(inputFile, outPrefix, 16, 1.6, 1080) ); + assertEquals( 1, enc.encode_fountain(inputFile, outPrefix, 16, 1.6, 1024) ); - uint64_t hash = 0xbdc232c714226fe6; + uint64_t hash = 0xa66a666543280e8e; std::string path = fmt::format("{}_0.png", outPrefix); cv::Mat img = cv::imread(path); - assertEquals( 1080, img.rows ); - assertEquals( 1080, img.cols ); + assertEquals( 1024, img.rows ); + assertEquals( 1024, img.cols ); assertEquals( hash, image_hash::average_hash(img) ); } diff --git a/src/lib/extractor/Scanner.h b/src/lib/extractor/Scanner.h index 10a44db3..3bf0a73b 100644 --- a/src/lib/extractor/Scanner.h +++ b/src/lib/extractor/Scanner.h @@ -2,6 +2,7 @@ #pragma once #include "Anchor.h" +#include "Corners.h" #include "Point.h" #include "ScanState.h" @@ -10,7 +11,6 @@ #include #include -class Corners; class Midpoints; class Scanner @@ -33,6 +33,10 @@ class Scanner static unsigned nextPowerOfTwoPlusOne(unsigned v); // helper + // external helper method + template + static bool will_it_scan(const MAT& img); + // rest of public interface std::vector scan(); std::vector> scan_edges(const Corners& corners, Midpoints& mps) const; @@ -98,6 +102,27 @@ inline unsigned Scanner::nextPowerOfTwoPlusOne(unsigned v) return std::max(3U, v + 2); } +template +inline bool Scanner::will_it_scan(const MAT& unpadded_img) +{ + Scanner scanner(unpadded_img); + std::vector points = scanner.scan(); + if (points.size() < 4) + return false; + + constexpr int limit = 50; + Corners corners(points); + if (corners.top_left().x() > limit or corners.top_left().y() > limit) + return false; + if (corners.top_right().x() < (unpadded_img.cols - limit) or corners.top_right().y() > limit) + return false; + if (corners.bottom_left().x() > limit or corners.bottom_left().y() < (unpadded_img.rows - limit)) + return false; + if (corners.bottom_right().x() < (unpadded_img.cols - limit) or corners.bottom_right().y() < (unpadded_img.rows - limit)) + return false; + return true; +} + template inline void Scanner::threshold_fast(const MAT& img, MAT2& out) { @@ -141,10 +166,10 @@ inline void Scanner::preprocess_image(const MAT& img, MAT2& out, bool fast) template inline Scanner::Scanner(const MAT& img, bool fast, bool dark, int skip) - : _dark(dark) - , _skip(skip? skip : std::min(img.rows, img.cols) / 60) - , _mergeCutoff(img.cols / 30) - , _anchorSize(30) + : _dark(dark) + , _skip(skip? skip : std::min(img.rows, img.cols) / 60) + , _mergeCutoff(img.cols / 30) + , _anchorSize(30) { _img = preprocess_image(img, fast); } From b97cedd57c6d70cf36af532c4451f7cb07f52330 Mon Sep 17 00:00:00 2001 From: sz Date: Tue, 5 Nov 2024 00:39:42 -0600 Subject: [PATCH 2/4] Sanity/bug check for safety scan failing during encode Ran some tests on how common this failure mode is, and 1-2%(!) of frames tend to be "bad" (and thus are thrown out). We should never ever hit this 5-in-a-row case, but it'd also be pretty brutal to infinite loop due to a bug in the will_it_scan() function... --- src/lib/encoder/Encoder.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/lib/encoder/Encoder.h b/src/lib/encoder/Encoder.h index 2115ab2f..fad1e685 100644 --- a/src/lib/encoder/Encoder.h +++ b/src/lib/encoder/Encoder.h @@ -56,6 +56,7 @@ inline unsigned Encoder::encode_fountain(const std::string& filename, const std: requiredFrames = 1; unsigned i = 0; + unsigned consecutiveScansFailed = 0; while (i < requiredFrames) { auto frame = encode_next(*fes, canvas_size); @@ -67,8 +68,15 @@ inline unsigned Encoder::encode_fountain(const std::string& filename, const std: // corner "anchors" and fail to extract. So: // if frame fails the scan, skip it. if (!Scanner::will_it_scan(*frame)) - continue; + { + if (++consecutiveScansFailed < 5) + continue; + // else, we gotta make forward progress. And it's probably a bug? + std::cerr << fmt::format("generated {} bad frames in a row. This really shouldn't happen, maybe report a bug. :(", consecutiveScansFailed) << std::endl; + } + + consecutiveScansFailed = 0; if (!on_frame(*frame, i)) break; ++i; From 8ec60b88faeb88391ee8064b93aa05ffb5ff3956 Mon Sep 17 00:00:00 2001 From: sz Date: Sun, 10 Nov 2024 23:59:04 -0600 Subject: [PATCH 3/4] Remove the logic to lock the web encoder to a single frame -- It modestly helps make decodes easier in the single frame case, but the 1-2% failures we see means we need to either: A. add the new scan sanity check. This might end up being the way to go, but it has performance drawbacks. B. remove the special case and generate some extra frames. Special cases are annoying anyway. --- src/lib/cimbar_js/cimbar_js.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/lib/cimbar_js/cimbar_js.cpp b/src/lib/cimbar_js/cimbar_js.cpp index 3ff7e478..889fe6fb 100644 --- a/src/lib/cimbar_js/cimbar_js.cpp +++ b/src/lib/cimbar_js/cimbar_js.cpp @@ -68,12 +68,11 @@ int next_frame() if (!_window or !_fes) return 0; - // we generate 5x the amount of required symbol blocks -- unless everything fits in a single frame. - // color blocks will contribute to this total, but only symbols are used for the initial calculation. - // ... this way, if the color decode is failing, it won't get "stuck" failing to read a single frame. - unsigned required = _fes->blocks_required(); - if (required > cimbar::Config::fountain_chunks_per_frame(cimbar::Config::symbol_bits(), _legacyMode)) - required = required*5; + // we generate 8x the amount of required symbol blocks. + // this number is somewhat arbitrary, but needs to not be + // *too* low (1-2), or we risk long runs of blocks the decoder + // has already seen. + unsigned required = _fes->blocks_required() * 8; if (_fes->block_count() > required) { _fes->restart(); From 6b540f42a44ac33e7f80a10dc160c9f8207b473d Mon Sep 17 00:00:00 2001 From: sz Date: Mon, 11 Nov 2024 01:29:48 -0600 Subject: [PATCH 4/4] Fix packaging script --- package-wasm.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-wasm.sh b/package-wasm.sh index d39df87b..48d4179c 100644 --- a/package-wasm.sh +++ b/package-wasm.sh @@ -16,7 +16,7 @@ mkdir build-wasm cd build-wasm emcmake cmake .. -DUSE_WASM=1 -DOPENCV_DIR=/usr/src/app/opencv4 make -j5 install -(cd ../web/ && tar -czvf cimbar.wasm.tar.gz cimbar_js.* index.html main.js) +(cd ../web/ && tar -czvf cimbar.wasm.tar.gz cimbar_js.js cimbar_js.wasm index.html main.js) cd /usr/src/app mkdir build-asmjs