Skip to content

Commit

Permalink
Switch ccm matrix to use a thread_local
Browse files Browse the repository at this point in the history
There are other ways to pass this around, but this is the simplest
change. The old way was a race condition in CFC. 😱

Probably will want to revisit this in the future. The shared global
state feels a big gross.
  • Loading branch information
sz3 committed Feb 5, 2024
1 parent f000b3d commit 6b6522a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
21 changes: 13 additions & 8 deletions src/lib/cimb_translator/CimbDecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,23 @@ CimbDecoder::CimbDecoder(unsigned symbol_bits, unsigned color_bits, unsigned col
load_tiles();
}

const color_correction& CimbDecoder::get_ccm() const
// protected
color_correction& CimbDecoder::internal_ccm() const
{
// testing purposes only.
// this returning a thread local would be fine, iff we only use it for debugging!
static thread_local color_correction _ccm;
return _ccm;
}

// public
const color_correction& CimbDecoder::get_ccm() const
{
// testing/debugging purposes only!!!!
return internal_ccm();
}

void CimbDecoder::update_color_correction(cv::Matx<float, 3, 3>&& ccm)
{
// TODO: threadlocal?
// because this is dubious to begin with...
_ccm.update(std::move(ccm));
internal_ccm().update(std::move(ccm));
}

uint64_t CimbDecoder::get_tile_hash(unsigned symbol) const
Expand Down Expand Up @@ -163,9 +168,9 @@ std::tuple<uchar,uchar,uchar> CimbDecoder::get_color(int i) const
unsigned CimbDecoder::get_best_color(float r, float g, float b) const
{
// transform color with ccm
if (_ccm.active())
if (internal_ccm().active())
{
std::tuple<float, float, float> color = _ccm.transform(r, g, b);
std::tuple<float, float, float> color = internal_ccm().transform(r, g, b);
r = std::get<0>(color);
g = std::get<1>(color);
b = std::get<2>(color);
Expand Down
3 changes: 2 additions & 1 deletion src/lib/cimb_translator/CimbDecoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class CimbDecoder
unsigned symbol_bits() const;

protected:
color_correction& internal_ccm() const;

uint64_t get_tile_hash(unsigned symbol) const;
bool load_tiles();

Expand All @@ -45,5 +47,4 @@ class CimbDecoder
unsigned _colorMode;
bool _dark;
uchar _ahashThreshold;
color_correction _ccm;
};
15 changes: 9 additions & 6 deletions src/lib/cimb_translator/test/CimbReaderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace {
{
public:
using CimbDecoder::CimbDecoder;
using CimbDecoder::_ccm;
using CimbDecoder::internal_ccm;
};
}
#include "serialize/str_join.h"
Expand Down Expand Up @@ -143,6 +143,7 @@ TEST_CASE( "CimbReaderTest/testCCM", "[unit]" )
cv::Mat sample = TestCimbar::loadSample("b/ex2434.jpg");

TestableCimbDecoder decoder(4, 2);
decoder.internal_ccm() = color_correction();
CimbReader cr(sample, decoder);

// this is the header value for the sample -- we could imitate what the Decoder does
Expand All @@ -151,10 +152,10 @@ TEST_CASE( "CimbReaderTest/testCCM", "[unit]" )
cr.update_metadata((char*)md.data(), md.md_size);
cr.init_ccm(2, cimbar::Config::interleave_blocks(), cimbar::Config::interleave_partitions(), cimbar::Config::fountain_chunks_per_frame(6, false));

assertTrue( decoder._ccm.active() );
assertTrue( decoder.get_ccm().active() );

std::stringstream ss;
ss << decoder._ccm.mat();
ss << decoder.get_ccm().mat();
assertEquals("[2.3991191, -0.41846275, -0.54654282;\n "
"-0.42976046, 2.632102, -0.76466882;\n "
"-0.54299992, -0.20199311, 2.2753253]", ss.str());
Expand All @@ -176,9 +177,10 @@ TEST_CASE( "CimbReaderTest/testCCM.Disabled", "[unit]" )
cv::Mat sample = TestCimbar::loadSample("b/ex2434.jpg");

TestableCimbDecoder decoder(4, 2);
decoder.internal_ccm() = color_correction();
CimbReader cr(sample, decoder, false, false);

assertFalse( decoder._ccm.active() );
assertFalse( decoder.get_ccm().active() );

std::array<unsigned, 6> expectedColors = {0, 1, 1, 2, 2, 2};
for (unsigned i = 0; i < expectedColors.size(); ++i)
Expand All @@ -197,6 +199,7 @@ TEST_CASE( "CimbReaderTest/testCCM.VeryNecessary", "[unit]" )
cv::Mat sample = TestCimbar::loadSample("b/ex380.jpg");

TestableCimbDecoder decoder(4, 2);
decoder.internal_ccm() = color_correction();
CimbReader cr(sample, decoder);

// this is the header value for the sample -- we could imitate what the Decoder does
Expand All @@ -205,10 +208,10 @@ TEST_CASE( "CimbReaderTest/testCCM.VeryNecessary", "[unit]" )
cr.update_metadata((char*)md.data(), md.md_size);
cr.init_ccm(2, cimbar::Config::interleave_blocks(), cimbar::Config::interleave_partitions(), cimbar::Config::fountain_chunks_per_frame(6, false));

assertTrue( decoder._ccm.active() );
assertTrue( decoder.get_ccm().active() );

std::stringstream ss;
ss << decoder._ccm.mat();
ss << decoder.get_ccm().mat();
assertEquals("[1.6250746, 0.0024788622, -0.45772526;\n "
"-0.29126319, 2.2922182, -0.67037439;\n "
"-1.2192062, -2.7447209, 5.0476217]", ss.str());
Expand Down

0 comments on commit 6b6522a

Please sign in to comment.