Misc bugfixes + dependency upgrades #59
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This requires some more thoughts -- the shape of the problem is that the encoder and decoder are running out of band with each other (very intentionally -- it's the whole point). We want to "keep up" with what the encoder throws at us, but we also need to clean up our list of in-progress (and finished!) decodes at some point -- we don't want it to be unbounded. The bug this fixes should manifest as files refusing to progress/download -- the decoder (app) has already reserved the "stream slot", finished a download, but failed to clean up the mess to let it be reclaimed. The workaround for CFC is to close and re-open the app, which will throw out all in progress downloads and start from a clean slate.
since the include path is different...
It's only needed for undistort. It be worth making optional...
Including my custom CMakeLists.
So I'm ok with cv3 and cv4 returning slightly different answers.
sz3
commented
Feb 19, 2022
| endif() | ||
|
|
||
| if(NOT DEFINED OPENCV_LIBS) | ||
| set(OPENCV_LIBS "opencv_core" "opencv_imgcodecs" "opencv_imgproc" "opencv_photo") | ||
| set(OPENCV_LIBS "opencv_calib3d" "opencv_imgcodecs" "opencv_imgproc" "opencv_photo" "opencv_core") |
There was a problem hiding this comment.
Ordering is helpful for static builds. calib3d is needed for opencv4 and the undistort functionality. undistort isn't currently used anywhere, but I didn't see a good reason to axe it yet.
sz3
commented
Feb 19, 2022
| @@ -1,8 +1,9 @@ | |||
| /* This code is subject to the terms of the Mozilla Public License, v.2.0. http://mozilla.org/MPL/2.0/. */ | |||
| #pragma once | |||
|
|
|||
| #include <vector> | |||
| #include <cstddef> | |||
sz3
commented
Feb 19, 2022
| @@ -33,7 +33,7 @@ class FountainMetadata | |||
| to_uint8_arr(encode_id, size, d); | |||
| } | |||
|
|
|||
| unsigned id() const | |||
| uint32_t id() const | |||
There was a problem hiding this comment.
This unsigned was doing a lot of work -- the type here should've been 32bit all along.
sz3
commented
Feb 19, 2022
| @@ -137,7 +137,10 @@ class fountain_decoder_sink | |||
| std::string _dataDir; | |||
| unsigned _chunkSize; | |||
|
|
|||
| // maybe instead of unordered_map+set, something where we can "age out" old streams? | |||
| // e.g. most recent 16/8, or something? | |||
| // question is what happens to _done/_streams when we wrap for continuous data streaming... | |||
There was a problem hiding this comment.
There's more work to be done here.
sz3
commented
Feb 19, 2022
| _done.insert(id); | ||
| auto it = _streams.find(id); | ||
| _done.insert(md.id()); | ||
| auto it = _streams.find(stream_slot(md)); |
There was a problem hiding this comment.
Because we were searching by id and (not by stream_slot), we'd never remove the old stream. Not great!
Luckily: this code is currently only used by the android app, and the whole sink gets thrown away whenever the app is stopped or paused.
Merged
sz3
added a commit
that referenced
this pull request
Mar 13, 2024
5fbface Merge pull request #60 from sz3/ci-package 9d52131 Draft a github release (with bits!) when a new tag is pushed 2010df7 It's still gcc9 1686eca Build on new ubuntu? 4ab275a a script to build cimbar.js bd87003 Ubuntu 16.04 build for glibc >= 2.23. Or 2.14(!?) 7bb1bc8 -DBUILD_PORTABLE_LINUX for static linking against opencv and libc++ 729eb7e Merge pull request #59 from sz3/bugfix-misc 9e0dae0 Compile time check is fine, I guess c05e3b5 hash difference aside, the binary differences are minor e698786 Update concurrentqueue (1.0.3) fec665f Update fmt (8.1.1) 0f2665a Upgrade intx cc6abee Update picosha2 and stb_image 39b37d9 Update wirehair 82ed9e9 oof 28559cb I think we need calib3d now? 178e02a Make CMakeLists smart enough to find opencv4 b0091d1 new catch2 to make compiler happy 52547ae Include to make g++11 happy 7c545bb cerr c5f7676 Bugfix (maybe): properly clean up old decodes f752be4 Merge pull request #58 from sz3/service-worker 19c7904 No PWA for now ef6df87 Merge pull request #57 from sz3/css-fix 8103bf0 Possibly functional? 74ceae3 add favicon... 8fa1509 Cache more stuff? clear old caches? e20a3cd WIP? cache please? 99eb349 Merge branch 'css-fix' into sw 8d4cbc9 Try using a button element? a7f663a Set zoom on the canvas element, not the whole page? d0dffa2 Slightly more responsive to window size changes 9e8e552 Fix the background css 989796b service worker attempt (doesn't work) git-subtree-dir: app/src/cpp/libcimbar git-subtree-split: 5fbface
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I left zstd as is -- the upgrade will need some further investigation.