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

DAS-2276: retain 3 and 4 band information in browse images #39

Merged
merged 18 commits into from
Dec 18, 2024

Conversation

flamingbear
Copy link
Member

@flamingbear flamingbear commented Dec 12, 2024

Description

Code is modified to change the handling of 3 and 4 band input GeoTIFFs.

convert_mulitband_to_raster is modified so that 4 band images are only modified:
1. If the original dtype is not 'uint8' (very rare) and the max valid value
exceeds 255, then the data is scaled from 0 to 255
2. If Rasterio picked up any missing values, they will be set to 0. This
shouldn't happen because 4 bands would use the alpha layer, but also, if
an rgb band has a nan, it's probably because the alpha is 0 (TRANSPARENT)

convert_mulitband_to_raster is modified so that 3-band images are modified:
1. To add a transparent alpha layer for any missing data values.
2. To scale if the input dtype is not 'uint8'

Jira Issue ID

DAS-2276

Local Test Steps

Before you build or test this PR do these steps first!

Pull current hybig docker image:

> docker pull ghcr.io/nasa/harmony-browse-image-generator:2.0.2

set Harmony-In-A-Box to use it by putting this in your harmony/.env file:

HYBIG_IMAGE=ghcr.io/nasa/harmony-browse-image-generator:2.0.2
HYBIG_SERVICE_QUEUE_URLS='["ghcr.io/nasa/harmony-browse-image-generator:2.0.2,http://sqs.us-west-2.localhost.localstack.cloud:4566/000000000000/hybig.fifo"]'

also enable opera-rtc-s1-browse by including it in your LOCALLY_DEPLOYED_SERVICES=..[snip]....,net2cog,harmony-smap-l2-gridder,opera-rtc-s1-browse

Pull the opera-rtc docker image (If you're on an M1 machine you'll need to force the amd64 image.):

docker pull --platform linux/amd64 ghcr.io/asfhyp3/opera-rtc-s1-browse:latest

Get on the vpn(?) and download this file, you may need to accept the EULA.
https://datapool-test.asf.alaska.edu/RTC/OPERA-S1/OPERA_L2_RTC-S1_T035-073251-IW2_20240512T020817Z_20240512T122756Z_S1A_30_v1.0_VV.tif

Fire up your Harmony-In-A-Box... Ensure the pods for opera-rtc-s1-browse are running correctly, and run the DAS-2276-validate.py script

copy the .png, .pgw and .png.aux.xml into an original directory.

Verify that the .png is paletted with gdalinfo, you should see a palette and one band:

<snip>
Center      (-122.3788455,  37.8443403) (122d22'43.84"W, 37d50'39.62"N)
Band 1 Block=4282x1 Type=Byte, ColorInterp=Palette
  Color Table (RGB with 256 entries)
    0: 255,255,255,255
    1: 248,255,248,255
    2: 237,255,237,255
    3: 227,255,227,255
<snip>

Now you can verify the PR

Pull this branch, build and run tests:

❯ ./bin/build-image && ./bin/build-test &&  ./bin/run-test

Update your harmony .env so Harmony-In-A-Box will run the new locally-built version of hybig:

HYBIG_IMAGE=ghcr.io/nasa/harmony-browse-image-generator:latest
HYBIG_SERVICE_QUEUE_URLS='["ghcr.io/nasa/harmony-browse-image-generator:latest,http://sqs.us-west-2.localhost.localstack.cloud:4566/000000000000/hybig.fifo"]'

Kill and restart Harmony-In-A-Box to pick up the changes.

Re-run the validation script. Copy the files to a different directory. run gdalinfo again to verify that the images are rgba banded.

<snip>
Center      (-122.3788455,  37.8443403) (122d22'43.84"W, 37d50'39.62"N)
Band 1 Block=4282x1 Type=Byte, ColorInterp=Red
  Mask Flags: PER_DATASET ALPHA
Band 2 Block=4282x1 Type=Byte, ColorInterp=Green
  Mask Flags: PER_DATASET ALPHA
Band 3 Block=4282x1 Type=Byte, ColorInterp=Blue
  Mask Flags: PER_DATASET ALPHA
Band 4 Block=4282x1 Type=Byte, ColorInterp=Alpha
<snip>

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • docker/service_version.txt updated if publishing a release.
  • Tests added/updated and passing.
  • Documentation updated (if needed).

3 and 4 band input no longer scales by default.

4 bands are just converted to uint8 (scaling if necessary)
3 bands add an alpha layer where there are missing values.
@flamingbear flamingbear marked this pull request as ready for review December 13, 2024 19:00
fix previous assumption that the 255 value is set to an RGBA value for
transparency. Now we are writing rgba without the quantization.
Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

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

I've left a couple of nits and questions. I'll try running the test instructions later this afternoon, though. Generally, this makes sense at first glance (beyond not quite knowing the initial need to make 3 or 4 band inputs not-paletted).

hybig/sizes.py Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
hybig/browse.py Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
hybig/browse.py Outdated Show resolved Hide resolved
hybig/browse.py Show resolved Hide resolved
hybig/browse.py Outdated Show resolved Hide resolved
tests/test_service/test_adapter.py Show resolved Hide resolved
hybig/browse.py Show resolved Hide resolved
@flamingbear
Copy link
Member Author

flamingbear commented Dec 17, 2024

need to make 3 or 4 band inputs not-paletted

@owenlittlejohns This was given to me as a GIBS requirement because of how they handle multiple images displayed at once. Alex explained it, but I cannot regurgitate it back to you. Other than it did make some sense in terms of the number of palettes they had to handle when displaying multiple images and how the quantization mechanism was generating a different palette for every image (as expected and designed)

Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

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

The code changes look good. I was unable to verify that the updated service runs as expected due to issues with the source data used in the test (this is being discussed internally in Slack).

Copy link
Collaborator

@lyonthefrog lyonthefrog left a comment

Choose a reason for hiding this comment

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

I just added a few questions and caught a few typos. I ran the test instructions (unit tests, opera request before and after this change), and got the expected results.

Pre-change output gdalinfo:
validate-pre-change
Post-change output gdalinfo:
validate-post-change
Unit tests:
unit-tests

CHANGELOG.md Show resolved Hide resolved
hybig/browse.py Show resolved Hide resolved
hybig/browse.py Outdated Show resolved Hide resolved
hybig/browse.py Outdated Show resolved Hide resolved
tests/unit/test_browse.py Show resolved Hide resolved
@flamingbear flamingbear merged commit 09081c9 into main Dec 18, 2024
7 checks passed
@flamingbear flamingbear deleted the mhs/DAS-2276/retain-rgba-bands-when-present branch December 18, 2024 23:17
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.

3 participants