-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add mode autodetection + update mode toggle to function as autodetect off/on #32
Conversation
f21b1a573 Merge pull request #94 from sz3/for-autodetect-decode f1e136f14 Decoder: when we known there's a mismatch between the fountain chunk size... f5607a376 Add test to validate we can survive giving the fountain sink the wrong data 7b11358d8 Use per call switch for color_mode -- rather than per object instance ae135499e Merge pull request #93 from sz3/old-app-hint-4c e560c0358 Switch cimbar_send to the old default (4C) to be consistent with cimbar_js 5e8a2a25f Switch encoder default back to mode 4C? d09bdda19 Use the menu to communicate that 4C is for backwards compatibility git-subtree-dir: app/src/cpp/libcimbar git-subtree-split: f21b1a573187526b566648024355f091fe83a6f3
The idea is (1) to allow the long-lived Decoder/CimbDecoder to switch between 4C/B at runtime, (2) give the decoder app a pseudo-mode that alternates between the two. When (2) succeeds in decoding in one of the modes, * we return which mode it was * switch the UI mode toggle graphic to reflect the detected mode * enable the mode toggle (which will now determine whether *autodetect* is active)
* unless we detect a mode change, keep the fountain_decoder_sink object around to preserve decoder progress. * put setChecked() the ui thread, so it won't throw
bool legacy_mode() const; | ||
int mode() const; | ||
bool set_mode(int mode_val); | ||
int detected_mode() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface is to allow us to switch modes while keeping the MultiThreadedDecoder around when the mode doesn't switch (rather than tearing it down, resetting progress). So we can click the toggle button to our heart's content...
@@ -87,7 +96,9 @@ inline int MultiThreadedDecoder::do_extract(const cv::Mat& mat, cv::Mat& img) | |||
|
|||
inline bool MultiThreadedDecoder::add(cv::Mat mat) | |||
{ | |||
return _pool.try_execute( [&, mat] () { | |||
++count; | |||
bool legacy_mode = _modeVal == 4 or (_modeVal == 0 and count%2 == 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crucially, we do this count -> legacy_mode calculation on the main thread, not on the background threads. 😶
return true; | ||
|
||
if (mode_val != 0 and _writer.chunk_size() != fountain_chunk_size(mode_val)) | ||
return false; // if so, we need to reset to change it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past (for example when from 4-color to 8-color, in 0.5.x), we had to throw away the MultiThreadedDecoder object and get a new one any time we changed anything. Since we have an "autodetect mode" now, we want to only do that when we must (since it currently requires throwing away progress).
The fountain_chunk_size being different is the condition. Which is to say, if the modes had the same fountain_chunk_size, we would be able to make continuous progress on the stream no matter what mode we were in. But there are other things that might prevent that, and in any case there are good reasons mode B
has to use a different number.
|
||
// reset detectedMode iff we're switching back to autodetect | ||
if (mode_val == 0) | ||
_detectedMode = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0
is our "it's not a real mode, it's autodetect mode" value.
@@ -205,6 +205,9 @@ Java_org_cimbar_camerafilecopy_MainActivity_processImageJNI(JNIEnv *env, jobject | |||
|
|||
// return a decoded file to prompt the user to save it, if there is a new one | |||
string result; | |||
if (proc->detected_mode()) // repurpose str for special message passing | |||
result = fmt::format("/{}", proc->detected_mode()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit ridiculous. But it also works. 🤔
if (res.startsWith("/")) { | ||
if (res.length() >= 2 && res.charAt(1) == '4') { | ||
detectedMode = 4; | ||
mModeSwitch.setActivated(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may regret this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to move it onto the UI thread. It's still a hack, but it feels slightly less bad now?
In this PR I left the old 4-color mode ("mode 4C") as the default, with the hope of making the upgrade process as painless as possible. To that end:
introducing mode autodetection!
B/4C
.This should all be pretty intuitive... as long as I didn't introduce any bugs in the process 🙃
Includes sz3/libcimbar#94