Rewrite of noaa_apt_sink
Rewrite of the noaa_apt_sink using the line synchronisation scheme from https://github.com/LongHairedHacker/apt-decoder/. The new implementation has been designed as a drop in replacement with the same interface. The code style/formating has been matched to the other source files in this repo.
Comments here or to ar3itrary in the matrix/irc-channel.
Differences to the previous implementation:
- Uses png++ instead of plain libpng
- Nicer interface and allowing for cleaner code
- libpng++ should be available on any standard linux distro
- fewer png internals exposed, less potential for mistakes
- Higher memory consumption as the whole image is kept in RAM (approx. 4MB)
- As png++ is only a wrapper around libpng, it consists solely of header files and does not provide its own shared object. Therefore it is only a build dependency required at compile time, not run time.
- More robust syncing code, ported from apt-decoder
- Uses a fuzzy matching/correlation to detect sync patterns
- Syncs on A and B patterns
- Resyncs twice on each line, if possible
- Considerably reduces slating (see image below)
- Tries to fill any gaps in the image generated with data from the modules sample history
- Images are written to disk every 250 lines, to have partial images even in case of crashes
added 10 commits
Toggle commit list
8dedb9b1 - 1 commit from branch
- 234a6cd6 - Added libpng++ to dependencies
- d15bf778 - Writing images, split and flip work
- dc2cb5ca - Syncing works
- 93236942 - Added minimal documentation
- 0e5aeb84 - Using stop instead of destructor for teardown
- 6cbe19ba - Readjust dynamic range after first sync
- a8442481 - Moved sync patterns into the class
- 5e895870 - Final cleanup and some more comments
- 7304a084 - Fixed file naming scheme
- 8dedb9b1 - 1 commit from branch
changed the descriptionToggle commit list
Hi @LongHairedHacker, that's a very nice contribution! Lets keep the discussion about the code here.
Some comments on your MR. I am a fun of C++11 but there is a debate about the
auto. Most of the community out there uses the
autofor iterators or range-based for loops etc. So I suggest to follow this scheme.
In addition is there any particular reason for setting as
size_t d_row_write_threshold? Just declare them
constand set them in the initialization list of the contructor.
added 11 commits
Toggle commit list
- 8acd0fe6 - Added libpng++ to dependencies
- 9d1567ac - Writing images, split and flip work
- db8d2a5e - Syncing works
- 1f3b50c9 - Added minimal documentation
- 612891e9 - Using stop instead of destructor for teardown
- 37d3dfeb - Readjust dynamic range after first sync
- 20d1c326 - Moved sync patterns into the class
- 88d3d06e - Final cleanup and some more comments
- a9118488 - Fixed file naming scheme
- 1baf103a - Merge branch 'noaa-apt-sink' of gitlab.com:LongHairedHacker/gr-satnogs into noaa-apt-sink
- abc385ae - Fixed code style issues
Okay I switched to using types instead of auto, to be honest I never quite understood to debate out there, the c++ standard is actually pretty clear on that one. It would have saved the hassle to go over the code again and fix all the temporary images and pixel types in case they ever need to be change. (And also it would have been implicitly clear, by the signature of the functions in use, that there is only one suitable type to chose.)
The static doesn't make difference here. In fact declaring them const and initializing them in the initialization list of the constructor will lead to the same code being generated in most cases. (At least for the code here.) I usually prefer my solution as it does not create the expression that the const variable are part of the classes underlying struct. (A habbit I picked up working on runtime libraries for uCs where the memory layout of classes was more important.) As it is more a matter of style in this case I changed it to const.
Size does not matter here so much. I prefer the initialization list, because you get a nice error (with the proper flags) if you forget to init one :D
Yeah okay. That's a actually a pretty good argument.
All ok! I will proceed with the merge and I will bump up the version.
mergedToggle commit list
approved this merge requestToggle commit list