Skip to content

Commit 4c6c37c

Browse files
committed
Switch ccm matrix to use a thread_local
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.
1 parent 6b8778e commit 4c6c37c

File tree

3 files changed

+24
-15
lines changed

3 files changed

+24
-15
lines changed

src/lib/cimb_translator/CimbDecoder.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,23 @@ CimbDecoder::CimbDecoder(unsigned symbol_bits, unsigned color_bits, unsigned col
6565
load_tiles();
6666
}
6767

68-
const color_correction& CimbDecoder::get_ccm() const
68+
// protected
69+
color_correction& CimbDecoder::internal_ccm() const
6970
{
70-
// testing purposes only.
71-
// this returning a thread local would be fine, iff we only use it for debugging!
71+
static thread_local color_correction _ccm;
7272
return _ccm;
7373
}
7474

75+
// public
76+
const color_correction& CimbDecoder::get_ccm() const
77+
{
78+
// testing/debugging purposes only!!!!
79+
return internal_ccm();
80+
}
81+
7582
void CimbDecoder::update_color_correction(cv::Matx<float, 3, 3>&& ccm)
7683
{
77-
// TODO: threadlocal?
78-
// because this is dubious to begin with...
79-
_ccm.update(std::move(ccm));
84+
internal_ccm().update(std::move(ccm));
8085
}
8186

8287
uint64_t CimbDecoder::get_tile_hash(unsigned symbol) const
@@ -163,9 +168,9 @@ std::tuple<uchar,uchar,uchar> CimbDecoder::get_color(int i) const
163168
unsigned CimbDecoder::get_best_color(float r, float g, float b) const
164169
{
165170
// transform color with ccm
166-
if (_ccm.active())
171+
if (internal_ccm().active())
167172
{
168-
std::tuple<float, float, float> color = _ccm.transform(r, g, b);
173+
std::tuple<float, float, float> color = internal_ccm().transform(r, g, b);
169174
r = std::get<0>(color);
170175
g = std::get<1>(color);
171176
b = std::get<2>(color);

src/lib/cimb_translator/CimbDecoder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ class CimbDecoder
3131
unsigned symbol_bits() const;
3232

3333
protected:
34+
color_correction& internal_ccm() const;
35+
3436
uint64_t get_tile_hash(unsigned symbol) const;
3537
bool load_tiles();
3638

@@ -45,5 +47,4 @@ class CimbDecoder
4547
unsigned _colorMode;
4648
bool _dark;
4749
uchar _ahashThreshold;
48-
color_correction _ccm;
4950
};

src/lib/cimb_translator/test/CimbReaderTest.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace {
2424
{
2525
public:
2626
using CimbDecoder::CimbDecoder;
27-
using CimbDecoder::_ccm;
27+
using CimbDecoder::internal_ccm;
2828
};
2929
}
3030
#include "serialize/str_join.h"
@@ -143,6 +143,7 @@ TEST_CASE( "CimbReaderTest/testCCM", "[unit]" )
143143
cv::Mat sample = TestCimbar::loadSample("b/ex2434.jpg");
144144

145145
TestableCimbDecoder decoder(4, 2);
146+
decoder.internal_ccm() = color_correction();
146147
CimbReader cr(sample, decoder);
147148

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

154-
assertTrue( decoder._ccm.active() );
155+
assertTrue( decoder.get_ccm().active() );
155156

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

178179
TestableCimbDecoder decoder(4, 2);
180+
decoder.internal_ccm() = color_correction();
179181
CimbReader cr(sample, decoder, false, false);
180182

181-
assertFalse( decoder._ccm.active() );
183+
assertFalse( decoder.get_ccm().active() );
182184

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

199201
TestableCimbDecoder decoder(4, 2);
202+
decoder.internal_ccm() = color_correction();
200203
CimbReader cr(sample, decoder);
201204

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

208-
assertTrue( decoder._ccm.active() );
211+
assertTrue( decoder.get_ccm().active() );
209212

210213
std::stringstream ss;
211-
ss << decoder._ccm.mat();
214+
ss << decoder.get_ccm().mat();
212215
assertEquals("[1.6250746, 0.0024788622, -0.45772526;\n "
213216
"-0.29126319, 2.2922182, -0.67037439;\n "
214217
"-1.2192062, -2.7447209, 5.0476217]", ss.str());

0 commit comments

Comments
 (0)