Skip to content
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 ability to read MIE4NITF files #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m-chaturvedi
Copy link

To read an MIE4NITF file, we do the following:

  1. In the project file, say test_project.prj, we specify the absolute path to an image list file.
  2. In the image list file, which is assumed to be in the same directory as the frames, we indicate MIE4NITF file using the prefix MIE4NITF:.

For example:

$ cat test_project.prj  
DataSetSpecifier=/home/chaturvedi/images/all_images.txt

-----

$ cat /home/chaturvedi/images/all_images.txt
frame__071.ntf
frame__072.ntf
frame__073.ntf
frame__074.ntf
frame__075.ntf
MIE4NITF:wpr-j2k.r0t0q0c1i1.NTF

-----

$ ls --format=single-column /home/chaturvedi/images/
all_images.txt
frame__001.ntf
frame__002.ntf
frame__071.ntf
frame__072.ntf
frame__073.ntf
frame__074.ntf
frame__075.ntf
wpr-j2k.r0t0q0c1i1.NTF

Note that /home/chaturvedi/images/ may contain more files than what are used.

Copy link
Member

@mwoehlke-kitware mwoehlke-kitware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I have only done a fairly superficial review... the style inconsistencies are rather distracting from the actual code. When adding code to any project, please try to follow the prevailing style of the surrounding code.

@@ -128,6 +134,57 @@ void vpFileDataSource::setMonitoringEnabled(bool enable)
}
}

bool add_mie4nitf_subdatasets (const QString &path, QStringList &list) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh... your code style is rather all over the place. Please try to conform to the existing style. (n.b. T& x, not T &x, and Allman braces. Fortunately this is not one of the files still using historic VTK-style braces. Your indenting is also inconsistent.)


if (!dataset)
{
qWarning().nospace()<< "GDAL could not load file." <<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is nospace() needed here? Also, please put << at the start of the next line, not the end of a line, and align with the prior <<.

if (!dataset)
{
qWarning().nospace()<< "GDAL could not load file." <<
path.toStdString().c_str();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides that this is the locale-incorrect way to convert to std::string (that would be stdString) or char* (that would be qPrintable), why are you doing this at all? Just pass the QString...


const QRegExp rgx = QRegExp("SUBDATASET_\\d+_NAME");

if(key.contains(rgx))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing braces.

@@ -154,21 +211,54 @@ void vpFileDataSource::update()
{
while (!file.atEnd())
{
const auto line = file.readLine();
QString line = file.readLine().trimmed();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't decrease use of AAA.

// use glob to get more than one frames, we use the prefix `MIE4NITF:`
// to indicate that this file contains more than one frame in NITF
// format.
const QString mie4nitf_prefix = "MIE4NITF:";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use QStringLiteral.

// For ex., in MIE4NITF, frame # 3 inside `/A.ntf` (0-indexed) is
// stored in:
// `NITF_IM:2:/A.ntf`.
QString path_to_check;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use camelCase for variable names.

@@ -138,6 +138,7 @@ void vtkGDALReader::vtkGDALReaderInternal::ReadMetaData(
this->ReleaseData();

this->GDALData = (GDALDataset*) GDALOpen(fileName.c_str(), GA_ReadOnly);
std::cout << "Using GDAL reader to open file: " << fileName << std::endl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how useful this is?

@m-chaturvedi
Copy link
Author

@mwoehlke-kitware, we have transferred the files to the following gitlab, I have addressed the comments there:
https://gitlab.kitware.com/LVMI/vivia/merge_requests/1/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants