Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support split encode/decoder mode, "mode B" #27

Merged
merged 33 commits into from
Feb 26, 2024
Merged

Support split encode/decoder mode, "mode B" #27

merged 33 commits into from
Feb 26, 2024

Conversation

sz3
Copy link
Owner

@sz3 sz3 commented Feb 22, 2024

counterpart to sz3/libcimbar#91 (and arguably sz3/libcimbar#92, though that one is low on code changes)

The motivation is to allow the symbol decode to occur independently of the color decode, allowing
(1) the decode to succeed -- at reduced capacity -- even if the color decode fails,
(2) the color decode to use the information it has from the symbol decode to generate a better color correction matrix, which will allow the color decode to succeed in a larger variety of conditions.

As an added bonus, the color decode can succeed while the symbol decode fails -- but in practice should not be common (in any case, it's not ideal).

This changeset reflects various experiments to that end:

  • should we continue to use the naive CCM in addition to a smarter method?

    • Answer: for now, no. We'll just use the new thing.
  • what should the "smarter method" be? The colour-science package uses a Moore–Penrose inverse of observed vs ideal color values. That seemed like a good starting point, but there were other questions:

    1. can we use kmeans-derived averages for our CCM? That would work with the old cimbar format, and no change would be required
      • A: we can, and it works! But we seem to get more consistent results with the format change.
      • ... which makes sense! We don't have any way to ground our kmeans averages to known truth with the old format, we're just hoping we pick correctly. (arguably this is not as scary as it seems -- we do have a pass/fail test at the end of all this, and we're checking at 15 frames-per-second)
      • ...that said, there are times where the kmeans clustering gets confused, and having a better way to get at the real values is always going to be superior.
    2. so, if we do change the format, can we get better averages for our "observed" values?
      • A. yes! We can use the the 6 header bytes (48 bits) for each fountain "chunk" in our symbol decode -- which happens before the color decode -- to predict what the fountain header bytes will be for the data encoded in our color bits. We also know where to look, and can sample these locations (with the expected data) to generate observed values for our expected colors.
    3. if the average of the "observed" values is good enough, do we need a CCM matrix at all? Can't we just use the observed values?
      • A: it sort of works, but mostly doesn't.
    4. can we/should we split the image into sectors, to potentially allow different target values to evaluate to different colors based on where they are in the image?
      • A: maybe? Results are mixed. You end up falling back to a global CCM sometimes. It's complicated.
      • this one may require some explanation, so here goes: the middle of the image tends to be brighter, with the edges being dimmer. So it could be beneficial to bin cells in a certain region (ex: maybe we have 2 regions, 1 as a radius around the center and 1 for everything else, or maybe we have 5, with a center region and 1 for each corner)
      • however, if we bin cells together, we're reducing the accuracy of our observed color values (since we'll have less sample data per bin). So it's not straightforward.
  • disclaimer: all of these methods are heuristics. Evaluation was done against real world captures of cimbar images (from different camera + in different conditions) to try to pick the best one, but simplicity of implementation also factored in. The main questions I was trying to answer were:

  1. is kmeans good enough? (or do we have to change the format?)
  2. do any of my clever ideas for splitting the CCM (or not using one) seem clearly better than the naive, single-shot "just use Moore-Penrose and forget about it"?

The answers, as far as I could tell, are (1) no and (2) probably no. (2) can be re-evaluated at any time, since it is purely implementation side.


The end result of all this is new 4-color 8x8 format called Mode B

(named after I made these changes, so currently a libcimbar-only name. I'll update this repo to use that convention eventually. Probably.)

Mode B is shorthand for "8x8, 4-color, 30/155ecc, 12 fountain blocks of 625 bytes apiece, split color and symbol decodes, using color set 1"

whereas Mode 4C (the original format) would be "8x8, 4-color, 30/155ecc, 10 fountain blocks of 750 bytes apiece, interleaved color and symbol decodes, using color set 0"

... which is why I decided to call it "B" and "C".

sz3 added 30 commits June 11, 2023 07:33
Also, *maybe* should switch to ecc_block_size=162 for 5x5, it lends
itself to more ecc flexibility.
However, in this case what I want to do is use kmeans not just for
analysis, but actually for the color decode itself...
Also, ditch the "relative_color_diff", I don't think it helps.
Also, some experimental stuff with the color scaling/threshholding
…imbar

Also *maybe* will be useful for the color decode (this is the real
reason to go through the trouble)
This is *mostly* analogous to what libcimbar is doing. (just much less
efficient)
The code is a bit messy, alas
This will (if it works) allow us to use the fountain header color cells
(which are interleaved through the image) as a sort of color key/legend
for the full decode step.

There is currently a bug, though...
one for the "center" area (determined by distance from middle of image),
one for the "edges" (everything else)

Quickly running into an adversarial worst case -- the default encode_id
is currently `0`, which results in a *lot* of 0 (green) color tiles and
very few of any other color. Meaning that we just don't have much data
for the other colors to safely split among multiple sections.
-- not sure what to do when the sampling data is bad, I *think* we might
just want to bail. (has implications for 8-color though (: )
Inclined to think the two-pass may be better, though
+ stick with bitstring 3.1.9 until I can look
at how the API changed in v4
@coveralls
Copy link

coveralls commented Feb 22, 2024

Pull Request Test Coverage Report for Build 7999867768

Details

  • -66 of 396 (83.33%) changed or added relevant lines in 11 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.5%) to 90.579%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cimbar/fountain/fountain_decoder_stream.py 3 5 60.0%
cimbar/grader.py 38 42 90.48%
cimbar/encode/cimb_translator.py 35 49 71.43%
cimbar/cimbar.py 166 212 78.3%
Files with Coverage Reduction New Missed Lines %
cimbar/deskew/scanner.py 1 96.96%
Totals Coverage Status
Change from base Build 5183822726: -0.5%
Covered Lines: 1769
Relevant Lines: 1953

💛 - Coveralls

@@ -22,10 +23,10 @@ def possible_colors(dark, bits=0):
]
elif dark and bits < 3:
colors = [
(0, 0xFF, 0),
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll need to add back the old color set in a follow up PR. I'm thinking of merging the dark flag into something like a color_mode flag. So we'd have 0 for the original color arrangement, 1 for this, and 100 (or something) for light mode.

# probably some scaling will be good.
def scale_color(self, r, g, b):
if self.disable_color_scaling:
return r, g, b
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code base continues to be a victim of being used for prototyping things. This variable and logic can probably be deleted...
... unless it ends up being really useful, so we'd better keep it 😶


def _correct_all_colors(self, r, g, b, sector):
if isinstance(self.ccm, list):
ccm = self.ccm[sector]
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this is another "well what if we want to..." -- in this case, using the naive ccm based on a single color average (white) followed by a von kries transform, then feeding those corrected values into the Moore-Penrose based CCM, then smashing the resulting CCMs together for one king of all CCMs.

Anyway I don't think this one is that useful at this point, and it's about as silly as that explanation makes it sound. So this one will probably get removed.

@@ -1,4 +1,5 @@
bitstring
bitstring==3.1.9
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point bitstring changed something and broke my stuff. I don't think it's a big deal, but it does need some attention.

This needs some cleanup. In any event, 0,1,3 are the options that matter
for now. I might put 2 (kmeans clustering) back at some point.
import bitstring
from bitstring import Bits, BitStream

# it'd be nice to use the frame id as well, but sometimes we skip frames.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting: I'm currently ignoring this advice in libcimbar and using the frame_id anyway. It's only going to mess up one decode, what's the harm? 😬

(TODO: do the math in libcimbar to not mess up that one frame)

else: # cc_setting == 3,5
ct.ccm = der.dot(ct.ccm)

if splits: # 6,7
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to come back to this at some point to see if there's something I missed. There's more work to be done here.

if ct.ccm is None or cc_setting == 4:
ct.ccm = der
else: # cc_setting == 3,5
ct.ccm = der.dot(ct.ccm)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This .dot nonsense needs to go away, probably. (merging the CCMs together)

yield -1, None

# state_info can be set at any time, but it will probably be set by the caller *after* the empty yield above
if state_info.get('color_correct') == 1:
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this got refactored, the kmeans code got left behind. It should probably make a comeback, because (1) it works, (2) it doesn't require a fountain decode to work, (3) it'll be useful as a comparison.

Just a bit of minor cleanup
@sz3 sz3 merged commit 7b0ab1d into master Feb 26, 2024
6 of 8 checks passed
@sz3 sz3 deleted the split-decodes branch February 26, 2024 02:06
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7999779292

Details

  • 327 of 391 (83.63%) changed or added relevant lines in 11 files are covered.
  • 12 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.5%) to 90.635%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cimbar/fountain/fountain_decoder_stream.py 3 5 60.0%
cimbar/grader.py 36 42 85.71%
cimbar/encode/cimb_translator.py 35 49 71.43%
cimbar/cimbar.py 165 207 79.71%
Files with Coverage Reduction New Missed Lines %
cimbar/deskew/scanner.py 1 96.96%
cimbar/cimbar.py 11 81.44%
Totals Coverage Status
Change from base Build 5183822726: -0.5%
Covered Lines: 1771
Relevant Lines: 1954

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jan 3, 2025

Pull Request Test Coverage Report for Build 7999635392

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 327 of 391 (83.63%) changed or added relevant lines in 11 files are covered.
  • 12 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.5%) to 90.583%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cimbar/fountain/fountain_decoder_stream.py 3 5 60.0%
cimbar/grader.py 36 42 85.71%
cimbar/encode/cimb_translator.py 35 49 71.43%
cimbar/cimbar.py 165 207 79.71%
Files with Coverage Reduction New Missed Lines %
cimbar/deskew/scanner.py 1 96.96%
cimbar/cimbar.py 11 81.44%
Totals Coverage Status
Change from base Build 5183822726: -0.5%
Covered Lines: 1770
Relevant Lines: 1954

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants