search

ngageoint / sarpy

A basic Python library to demonstrate reading, writing, display, and simple processing of complex SAR data using the NGA SICD standard.
MIT License
246 stars 85 forks source link

Inconsistency in CSIDD Bits Per Pixel #544

Open klucar opened 1 week ago

klucar commented 1 week ago

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

https://github.com/ngageoint/sarpy/blob/3354207e6e8d8920416d1248c9c626c7d5a86b47/sarpy/io/product/sidd.py#L751

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

https://github.com/ngageoint/sarpy/blob/3354207e6e8d8920416d1248c9c626c7d5a86b47/sarpy/consistency/sidd_consistency.py#L350

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

bombaci-vsc commented 1 week ago

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 commented 1 week ago

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 commented 6 days ago

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

pressler-vsc commented 6 days ago

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 commented 6 days ago

Here you go!

https://github.com/ngageoint/sarpy/pull/545