-
Notifications
You must be signed in to change notification settings - Fork 34
Code Review meeting Jan 2016
Nitf (https://github.com/codice/imaging-nitf/blob/master/core/src/main/java/org/codice/imaging/nitf/core/Nitf.java) doesn't really say it is file header. Possibly rename?
There are a range of other files that have Nitf prefixes, but if there was ever a rule for what belonged where, its long gone. Remove all the prefixes (and infixes)? Could be done as part of the repackaging work.
See https://github.com/codice/imaging-nitf/tree/writing for the main work I also have some more experimental stuff on https://github.com/bradh/imaging-nitf/tree/writehacks
There are a range of fixes and API changes that I'll look to merge once the main repackaging work is finished.
I have "write to filename" and "write to stream" writers. They both wrap a DataOutputStream shared writer that does all the work. Anything else we might need?
We need something to read data from, for writing, and as an interface for applications. Right now that is the SlottedNitfParseStrategy, which is a big class, and only works if you are parsing (not constructing anew). I extracted a NitfDataSource interface: https://github.com/bradh/imaging-nitf/blob/writehacks/core/src/main/java/org/codice/imaging/nitf/core/NitfDataSource.java I can live with the basic header stuff (IEnumerable instead of List, perhaps), but the data interfaces may need more work, because they pre-suppose you have the entire thing in a set of byte[].
It is very optimistic to hope that a user can get a sensible structure without something to work from. The most likely case is probably an existing file, which we can handle. However there might be cases where its desirable to construct a NITF file from something else (like an image in another format). If we are going to be able to add TRE or additional segments, they have to come from something.
I considered providing an XML structure to read, but in the end, "give me sensible defaults" looks better. In terms of API, I've implemented a static method: https://github.com/bradh/imaging-nitf/blob/writehacks/core/src/main/java/org/codice/imaging/nitf/core/Nitf.java#L471 but that isn't sitting well. Perhaps a constructor would be better, or just making sure initial values are always sensible (which might get tricky in the NITF 2.10 to 2.11 transition)?
Update: On merging the IMG-50 changes, it gets really ugly because the interfaces can't contain static factory methods or constructors. So we'll likely need to introduce factory classes.
Right now we have a variety of lists in the slotted parser implementation. In particular, headers and data for each segment are kept in separate, parallel lists. Its important that they are the same length (and same order), and right now they aren't always (for DES). The fix isn't too bad, but it possibly indicates an internal design flaw.
The API version of that is about what the "high level" API will be for adding or removing segments from an existing or newly constructed file. Need advice on that, but may defer for further investigation.
I have some really large test files from the JITC RSM set - 90MB each. Github warns on committing those, and suggests LFS. 100MB is apparently the hard limit, and we'll eventually want to have very large tests (if only to check we don't blow up on them, but there will likely be functionality that requires multi GB tests). Is LFS OK? I think I'll split shared-test-resources off into a separate repo, and have a dependency on a fixed (not snapshot) release. At least the RSM files will zip up really well.
We have slotted parser variations to pull out just about any combination of headers and data. Are they really worth keeping? Perhaps we can have a constructor that takes a enumeration set (flags) to specify behaviour?