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

Inconsistency in CSIDD Bits Per Pixel #544

Open
klucar opened this issue Sep 10, 2024 · 5 comments
Open

Inconsistency in CSIDD Bits Per Pixel #544

klucar opened this issue Sep 10, 2024 · 5 comments
Labels
bug Something isn't working in integration merged into integration branch

Comments

@klucar
Copy link

klucar commented Sep 10, 2024

When writing a CSIDD in RGB24I data format, SarPy sets the number of bits per pixel (NBPP) to 8

basic_args['NBPP'] = 8

However, in the consistency checker, the number of bits it is looking for is 24.

sidd_nitf_details.append({'nbpp': 24, 'pvtype': 'INT', 'pixel_type': the_sidd.Display.PixelType})

My thought is that the former needs to be 24 instead of 8.

@bombaci-vsc
Copy link
Contributor

I believe the consistency checker has the error. The NBPP field's full name is "Number of Bits Per Pixel Per Band". 24bit RGB images are stored as three 8bit bands (NBANDS=3, NBPP=8).

The NITF standard is hard to follow, and the exact wording has changed between the MIL-STD-2500C and BIIF documents. However, I think they all restrict NBPP to 8, 16, 32, and 64 for integer pixel types. Here's the 2500C, which may be the most concise:

image

@pressler-vsc pressler-vsc added the bug Something isn't working label Sep 11, 2024
@pressler-vsc
Copy link
Collaborator

Thanks @klucar and @bombaci-vsc; it does like there is a bug here in SarPy.

sarpy.io.product.sidd seems consistent with SIDD Volume 2:

image

Table 2-6 doesn't really clarify, but the text above it appears consistent:

image

Given this, I too think the bug is in the consistency checker.

@klucar
Copy link
Author

klucar commented Sep 12, 2024

Looks like I guessed wrong! Want me to do a pull request or will this be fixed elsewhere?

@pressler-vsc
Copy link
Collaborator

Looks like I guessed wrong! Want me to do a pull request or will this be fixed elsewhere?

I don't believe this is being currently worked and PRs are always welcome! This is a good opportunity/reminder to get at least token sidd_consistency coverage in a unittest.

@klucar
Copy link
Author

klucar commented Sep 12, 2024

Here you go!

#545

@pressler-vsc pressler-vsc added the in integration merged into integration branch label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in integration merged into integration branch
Projects
None yet
Development

No branches or pull requests

3 participants