Add SICD byte provider #202

asylvest · 2017-09-10T16:21:35Z

This adds functionality that allows the caller to provide an AOI of SICD pixel data and get back corresponding raw NITF file bytes in a contiguous block of memory such that, if it's called eventually with the entire image, the caller will get the entire raw NITF file (including NITF headers). The idea is to make it simple for callers to write out a SICD NITF piecemeal without needing to understand the underlying complexities of the NITF file structure.

…s buffer, want this to be a series of pointers that are as contiguous as they can be. Also providing the offset in the file.

asylvest · 2017-09-11T01:43:18Z

There is a bit more functionality I think should be added as part of this PR... you can hold off on reviewing for now - I'll let you know when it's ready.

…range of rows

JonathanMeans

Leave a comment

JonathanMeans · 2017-09-13T11:26:34Z

six/modules/c++/six.sicd/include/six/sicd/SICDByteProvider.h

+     * \brief Represents a sequence of buffers which appear in contiguous order
+     * in the NITF (the underlying pointers are not contiguous)
+     */
+    struct BufferList


Could this be renamed to NITFBufferList or something? I don't think this could be truly ambiguous, but I still don't like that it overlaps with the typedef in NITFWriteControl.

Yeah that is a good idea - renamed to NITFBuffer and NITFBufferList

JonathanMeans · 2017-09-13T11:51:49Z

six/modules/c++/six.sicd/include/six/sicd/SICDByteProvider.h

+}
+}
+
+#endif


newline at end of file

Are you sure on these? Eclipse on both Linux and Windows is showing that newline... does it just look like that on the GitHub site?

Hm, you're right. I think I've been adding an extra newline to the end of everything...

JonathanMeans · 2017-09-13T11:52:04Z

six/modules/c++/six.sicd/source/SICDByteProvider.cpp

+    }
+}
+}
+}


needs newline

JonathanMeans · 2017-09-13T11:52:17Z

six/modules/c++/six.sicd/tests/TestUtilities.h

+    const std::string mPathname;
+};
+
+#endif


needs newline

JonathanMeans · 2017-09-13T12:06:31Z

six/modules/c++/six/source/NITFSegmentInfo.cpp

+        return false;
+    }
+}
+}


needs newline

JonathanMeans · 2017-09-13T12:07:05Z

six/modules/c++/six.sicd/tests/test_sicd_byte_provider.cpp

+        std::cerr << "Caught unknown exception\n";
+        return 1;
+    }
+}


needs newline

JonathanMeans · 2017-09-13T12:16:01Z

six/modules/c++/six.sicd/tests/test_streaming_write.cpp

@@ -1,10 +1,10 @@
 /* =========================================================================


I'm getting a compile error at line 154 on Linux. I think it doesn't like getting std::auto_ptr<six::sicd::ComplexData> instead of std::auto_ptr<six::Data>.

../six/modules/c++/six.sicd/tests/test_streaming_write.cpp: In instantiation of ‘void {anonymous}::Tester<DataTypeT>::normalWrite() [with DataTypeT = fl oat]’: ../six/modules/c++/six.sicd/tests/test_streaming_write.cpp:87:21: required from ‘{anonymous}::Tester<DataTypeT>::Tester(const std::vector<std::basic_string<char> >&, bool, size_t) [with DataTypeT = float; size_t = long unsigned int]’ ../six/modules/c++/six.sicd/tests/test_streaming_write.cpp:304:76: required from ‘bool {anonymous}::doTests(const std::vector<std::basic_string<char> >&, bool, size_t) [with DataTypeT = float; size_t = long unsigned int]’ ../six/modules/c++/six.sicd/tests/test_streaming_write.cpp:317:70: required from here ../six/modules/c++/six.sicd/tests/test_streaming_write.cpp:154:5: error: no matching function for call to ‘std::auto_ptr<six::Data>::auto_ptr(std::auto_ptr<six::Data>)’ mContainer->addData(createData<DataTypeT>(mDims));

Ahh that's lame (Visual Studio figures it out), but thanks for letting me know - releasing the auto_ptr now so it can just do a simple pointer conversion (container will still take ownership).

…licitly

asylvest · 2017-09-14T02:37:04Z

FYI, there's still one more piece I need to add in here that I haven't had a chance to get to yet.

…of bytes, avoiding copies where possible.

… getBlock()

asylvest · 2017-09-16T18:00:13Z

OK, this finally has everything I want to throw in here... @JonathanMeans go ahead and review when you have a chance.

JonathanMeans

Looks good.

c2489fe Merge pull request #208 from mdaus/polyChecks 5b6a037 Add check for Poly1D.fromArray 3e68b18 Add some safety checks when converting Polys to and from numpy arrays 41065e2 Merge pull request #206 from mdaus/remove_curl_env_pollution 2eeca61 Merge pull request #207 from mdaus/fix_include_guard 3a23d69 Fixing include guard 0682acc Need to undefine HAVE_CURL_GLOBAL_INIT or else it ends up in the waf environment 81002ad Merge pull request #205 from mdaus/cliErrorMessage 576f028 More verbose error message for invalid cli key d551442 Merge pull request #204 from mdaus/libcurl-link d54f4de Some systems require explicitly adding curl to link line 30de0ce Merge pull request #202 from mdaus/buildRefactor 499ef22 Merge pull request #203 from mdaus/install-tgt-fix 7954e52 Updated install_tgt after testing with --install-source dadd7a1 Changed input to be self instead of tsk c66b774 install_tgt does not have self as an input parameter f5d98e2 Merge pull request #201 from mdaus/pyInitFix 1a18f65 Extract another function; resolve targets_to_add/targetsToAdd naming inconsistency bdbac8f Explain the fix 5fd1058 More built-in way of guarding against double-installation 4408d50 Include whitespace change 7050277 Merge branch 'master' into pyInitFix 3a8b8b8 Only create one __init__.py per project, to prevent race conditions 2f2c23f Merge pull request #200 from mdaus/revertInit 3485d90 Revert __init__.py change bcd3b41 Merge pull request #198 from mdaus/pyInitFix d1b2a32 Merge pull request #199 from mdaus/buildRefactor 7448249 Pass an argument instead of modifying modArgs d42db38 Properly modify modArgs 7ccc3b4 Extract some repeated code into functions 99d3be0 Remove check that __init__ isn't install 884ad04 Merge pull request #197 from mdaus/pythonTestTarget 688ecf5 Python tests only install when asked, and when everything is installed 9084676 Fix bug in creating __init__.py 8e1b43b Clean up be131fb Prevent installation of python tests if not requested 9d24cdd Merge pull request #196 from mdaus/fixImports f92b113 Fix imports 70b8180 Merge pull request #194 from mdaus/mathUtils 911e578 Review fixes d8444d9 Merge pull request #195 from mdaus/add-curl-handle fcbfb72 Added a newline to the end of file 2746903 Moved includes to be inside the #ifdef check 6f81af1 Added check for CURL 63d6634 Added curl handle ff9dd87 Review fixes 3c67452 Add trailing newline c2b09e8 Add nChooseK function f2d4539 Merge pull request #193 from mdaus/matlabConfigFix e290bdb Continue config if can't find mexext 3f2a7ca Merge pull request #192 from mdaus/fix_gcc62 bd8c12b Finalize fixes 053b7be Add untested attempt at expanding symlinks 86b0742 Not sure why this used to be linux/unistd.h - we should be able to use unistd.h directly. gcc 6.2 and/or Alpine Linux can't find linux/unistd.h so switching this. 98f5f0f gcc 6.2 warns that this should have a cast - according to the man page, this is true git-subtree-dir: externals/coda-oss git-subtree-split: c2489fe74942f5a134f814a7335eec0375af5319

2645b4f Merge pull request #211 from mdaus/logging-improvements 268e38b Added mClosed to init list 3070346 Updated handlers to avoid calling virtual methods in the destructor 6fe1538 Merge pull request #210 from mdaus/vectorString ca69675 Move VectorString to coda_types 55e9b33 Merge pull request #209 from mdaus/nanCheck 01df546 Add copyright 0ce5221 Make some test macros work against NaNs c2489fe Merge pull request #208 from mdaus/polyChecks 5b6a037 Add check for Poly1D.fromArray 3e68b18 Add some safety checks when converting Polys to and from numpy arrays 41065e2 Merge pull request #206 from mdaus/remove_curl_env_pollution 2eeca61 Merge pull request #207 from mdaus/fix_include_guard 3a23d69 Fixing include guard 0682acc Need to undefine HAVE_CURL_GLOBAL_INIT or else it ends up in the waf environment 81002ad Merge pull request #205 from mdaus/cliErrorMessage 576f028 More verbose error message for invalid cli key d551442 Merge pull request #204 from mdaus/libcurl-link d54f4de Some systems require explicitly adding curl to link line 30de0ce Merge pull request #202 from mdaus/buildRefactor 499ef22 Merge pull request #203 from mdaus/install-tgt-fix 7954e52 Updated install_tgt after testing with --install-source dadd7a1 Changed input to be self instead of tsk c66b774 install_tgt does not have self as an input parameter f5d98e2 Merge pull request #201 from mdaus/pyInitFix 1a18f65 Extract another function; resolve targets_to_add/targetsToAdd naming inconsistency bdbac8f Explain the fix 5fd1058 More built-in way of guarding against double-installation 4408d50 Include whitespace change 7050277 Merge branch 'master' into pyInitFix 3a8b8b8 Only create one __init__.py per project, to prevent race conditions 2f2c23f Merge pull request #200 from mdaus/revertInit 3485d90 Revert __init__.py change bcd3b41 Merge pull request #198 from mdaus/pyInitFix d1b2a32 Merge pull request #199 from mdaus/buildRefactor 7448249 Pass an argument instead of modifying modArgs d42db38 Properly modify modArgs 7ccc3b4 Extract some repeated code into functions 99d3be0 Remove check that __init__ isn't install 884ad04 Merge pull request #197 from mdaus/pythonTestTarget 688ecf5 Python tests only install when asked, and when everything is installed 9084676 Fix bug in creating __init__.py 8e1b43b Clean up be131fb Prevent installation of python tests if not requested 9d24cdd Merge pull request #196 from mdaus/fixImports f92b113 Fix imports 70b8180 Merge pull request #194 from mdaus/mathUtils 911e578 Review fixes d8444d9 Merge pull request #195 from mdaus/add-curl-handle fcbfb72 Added a newline to the end of file 2746903 Moved includes to be inside the #ifdef check 6f81af1 Added check for CURL 63d6634 Added curl handle ff9dd87 Review fixes 3c67452 Add trailing newline c2b09e8 Add nChooseK function f2d4539 Merge pull request #193 from mdaus/matlabConfigFix e290bdb Continue config if can't find mexext 3f2a7ca Merge pull request #192 from mdaus/fix_gcc62 bd8c12b Finalize fixes 053b7be Add untested attempt at expanding symlinks 86b0742 Not sure why this used to be linux/unistd.h - we should be able to use unistd.h directly. gcc 6.2 and/or Alpine Linux can't find linux/unistd.h so switching this. 98f5f0f gcc 6.2 warns that this should have a cast - according to the man page, this is true 7c39239 Merge pull request #191 from mdaus/swig-python-extra-native-containers ec4d5f1 defaulting SWIG_PYTHON_EXTRA_NATIVE_CONTAINERS 6dcac97 Merge pull request #190 from mdaus/thread_planner b26de0e Member initialization should be on separate lines d71d3ed Hadn't clicked save before pushing - some more Doxygen comments 860adee Adding a new method to get the number of threads that will actually be used. Adding Doxygen. 85a2433 Merge pull request #189 from mdaus/cleanup b6ec743 Remove unused variables dba6ab3 Merge pull request #187 from mdaus/tempfile 64648fa Prevent memory leak on error; make tempfile uncopyable, unassignable 2e8229f Remove warning; use vector<char> instead of array 8555040 Remove unneeded include 8b17367 Remove option to not destroy tempfile; document getTempFile() side-effect b264832 Move TempFile to io; simply things a0107a0 Add TempFile class git-subtree-dir: externals/coda-oss git-subtree-split: 2645b4f9b008fc219d41f1b1b5c0170bf9aae949

asylvest added 7 commits September 10, 2017 09:46

Added convenience method to provide the end row

334787b

Providing NITFSegmentInfo in getFileLayout() too

0f611b4

Adding SICDByteProvider

9610fe3

Moving common testing functions/classes out to their own header

78779b1

Allowing the max product size to be overridden

db43347

Adding test for SICDByteProvider

7b48085

Adding SICD byte provider test

4ec4160

asylvest added this to the SIX 2.2.8 milestone Sep 10, 2017

asylvest requested a review from JonathanMeans September 10, 2017 16:21

Might want to pass in a ton of image data - rather than one contiguou…

e96ca5a

…s buffer, want this to be a series of pointers that are as contiguous as they can be. Also providing the offset in the file.

asylvest changed the title ~~Add SICD byte provider~~ WIP: Add SICD byte provider Sep 11, 2017

asylvest added 3 commits September 12, 2017 21:10

Added method to determine when a range of rows overlaps with a segment

72ec21c

Moving logic for determining segment overlap into NITFSegmentInfo

a4d8b16

Adding getNumBytes() method that determines the number of bytes in a …

e0529ad

…range of rows

JonathanMeans reviewed Sep 13, 2017

View reviewed changes

asylvest added 3 commits September 13, 2017 22:24

Renaming Buffer/BufferList to NITFBuffer/NITFBufferList

54ca92a

Removing unused variable

ccb85fd

gcc couldn't convert from auto_ptr<ComplexData> to auto_ptr<Data> imp…

84bf1c0

…licitly

asylvest added 3 commits September 14, 2017 21:22

Adding methods to provide the contents of a NITFBufferList in blocks …

687d237

…of bytes, avoiding copies where possible.

Adding getNumBytesInBlock() and providing back the number of bytes in…

5937f25

… getBlock()

Adding unit tests for getBlock()

211dc9c

asylvest changed the title ~~WIP: Add SICD byte provider~~ Add SICD byte provider Sep 16, 2017

JonathanMeans approved these changes Sep 18, 2017

View reviewed changes

asylvest merged commit 8ead9bb into master Sep 18, 2017

asylvest deleted the sicd_byte_provider branch September 18, 2017 12:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SICD byte provider #202

Add SICD byte provider #202

asylvest commented Sep 10, 2017

asylvest commented Sep 11, 2017

JonathanMeans left a comment

JonathanMeans Sep 13, 2017

asylvest Sep 14, 2017

JonathanMeans Sep 13, 2017

asylvest Sep 14, 2017

JonathanMeans Sep 14, 2017

JonathanMeans Sep 13, 2017

JonathanMeans Sep 13, 2017

JonathanMeans Sep 13, 2017

JonathanMeans Sep 13, 2017

JonathanMeans Sep 13, 2017

asylvest Sep 14, 2017

asylvest commented Sep 14, 2017

asylvest commented Sep 16, 2017

JonathanMeans left a comment

+              }
+              }
+              #endif

		@@ -1,10 +1,10 @@
		/* =========================================================================

Add SICD byte provider #202

Add SICD byte provider #202

Conversation

asylvest commented Sep 10, 2017

asylvest commented Sep 11, 2017

JonathanMeans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asylvest commented Sep 14, 2017

asylvest commented Sep 16, 2017

JonathanMeans left a comment

Choose a reason for hiding this comment