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

generic_image reader returns data as float64 for PNG images #2897

Closed
strandgren opened this issue Sep 9, 2024 · 19 comments
Closed

generic_image reader returns data as float64 for PNG images #2897

strandgren opened this issue Sep 9, 2024 · 19 comments

Comments

@strandgren
Copy link
Collaborator

Describe the bug
The generic_image reader returns the image data as float64 for PNG images. This leads to corrupt images with the PNG and JPEG writers when trying to save the image back to a file again, while the geotiff writer falls over completely when trying to save the image back to a TIF file.

To Reproduce

from satpy import Scene

infiles = ['input.jpg', 'input.png']
outfiles =  ['output.jpg', 'output.png']
for infile, outfile in zip(infiles, outfiles):
    scn = Scene(filenames=[infile], reader='generic_image')
    scn.load(['image'])
    print(scn['image'].dtype)
    scn.save_dataset('image', outfile, enhance=False, fill_value=0)

Expected behavior

uint8
uint8

and one copy each of the input images.

Actual results

uint8
float64

and a corrupt output.png image as shown in the screenshot below.
image

Environment Info:

  • OS: Linux
  • Satpy Version: 0.51.0

Additional context
When trying to save input.png as output.tif the geotiff writer fails with the following traceback:

Traceback (most recent call last):
  File "/tcenas/home/strandgren/git/eum/py/mtgi_imagery_stream/te_generic_reader.py", line 9, in <module>
    scn.save_dataset('image', outfile, enhance=False, fill_value=0)
  File "/tcenas/proj/optcalimg/miniconda3/envs/pysat/lib/python3.10/site-packages/satpy/scene.py", line 1234, in save_dataset
    return writer.save_dataset(self[dataset_id],
  File "/tcenas/proj/optcalimg/miniconda3/envs/pysat/lib/python3.10/site-packages/satpy/writers/__init__.py", line 885, in save_dataset
    return self.save_image(img, filename=filename, compute=compute, fill_value=fill_value, **kwargs)
  File "/tcenas/proj/optcalimg/miniconda3/envs/pysat/lib/python3.10/site-packages/satpy/writers/geotiff.py", line 263, in save_image
    raise ValueError("Image must be in 'L' mode for floating "
ValueError: Image must be in 'L' mode for floating point geotiff saving
@djhoese
Copy link
Member

djhoese commented Sep 11, 2024

If you run gdalinfo input.png, what do you get?

@djhoese
Copy link
Member

djhoese commented Sep 11, 2024

Ah it looks like this is intentional for fill/mask handling:

def _mask_image_data(data, info):
"""Mask image data if necessary.
Masking is done if alpha channel is present or
dataset 'nodata_handling' is set to 'nan_mask'.
In the latter case even integer data is converted
to float32 and masked with np.nan.
"""
if data.bands.size in (2, 4):
if not np.issubdtype(data.dtype, np.integer):
raise ValueError("Only integer datatypes can be used as a mask.")
mask = data.data[-1, :, :] == np.iinfo(data.dtype).min
data = data.astype(np.float64)
masked_data = da.stack([da.where(mask, np.nan, data.data[i, :, :])
for i in range(data.shape[0])])
data.data = masked_data
data = data.sel(bands=BANDS[data.bands.size - 1])
elif hasattr(data, "nodatavals") and data.nodatavals:
data = _handle_nodatavals(data, info.get("nodata_handling", NODATA_HANDLING_FILLVALUE))
return data

CC @ch-k @mraspaud

@djhoese
Copy link
Member

djhoese commented Sep 11, 2024

See #1560

@strandgren
Copy link
Collaborator Author

If you run gdalinfo input.png, what do you get?

!$ gdalinfo input.png
Driver: PNG/Portable Network Graphics
Files: input.png
Size is 11136, 11136
Image Structure Metadata:
  INTERLEAVE=PIXEL
Corner Coordinates:
Upper Left  (    0.0,    0.0)
Lower Left  (    0.0,11136.0)
Upper Right (11136.0,    0.0)
Lower Right (11136.0,11136.0)
Center      ( 5568.0, 5568.0)
Band 1 Block=11136x1 Type=Byte, ColorInterp=Red
  Mask Flags: PER_DATASET ALPHA
Band 2 Block=11136x1 Type=Byte, ColorInterp=Green
  Mask Flags: PER_DATASET ALPHA
Band 3 Block=11136x1 Type=Byte, ColorInterp=Blue
  Mask Flags: PER_DATASET ALPHA
Band 4 Block=11136x1 Type=Byte, ColorInterp=Alpha

I see, so then this is expected. However, is there a way to avoid broken output when saving to PNG and failing writing when trying to save to tif? I.e. can I provide some argument to save.datasets?

For my application it's easy enough to change the data type to uint8 before saving the image, but if there is a dedicated solution while writing that would be the cleaner option.

@djhoese
Copy link
Member

djhoese commented Sep 13, 2024

I don't think so. I kind of feel like my preference would be for an option to generic_image to not auto-mask loaded data based on alpha or other TIFF properties. Put another way, if I gave you an image, give me the image data. @ch-k, @pnuu, and @mraspaud What do you think?

@pnuu
Copy link
Member

pnuu commented Sep 16, 2024

What happens if you set enhance=True, does the saving work? The data might be re-stretched, but I hope the saving will then work.

I think having the kwarg @djhoese suggested above to keep the dtype intact for the input image makes a lot of sense.

I'm not sure why the image date were converted to float64 when the docstring specifically says float32.

@strandgren
Copy link
Collaborator Author

What happens if you set enhance=True, does the saving work? The data might be re-stretched, but I hope the saving will then work.

I think having the kwarg @djhoese suggested above to keep the dtype intact for the input image makes a lot of sense.

I'm not sure why the image date were converted to float64 when the docstring specifically says float32.

Yes, with enhance=True it works, but indeed with the caveat that the image is re-stretched, which I wont to avoid for my application (which is to load static JPG/PNG images, assign them a known area definition and then resample/crop to a new area and save the image back to a static image file).

@pnuu
Copy link
Member

pnuu commented Sep 16, 2024

The next workaround to test:
Create $SATPY_CONFIG_PATH/enhancements/images.yaml with

enhancements:
  default:
    operations: []

to override the default enhancement in generic.yaml only for sensor: images and then try with enhance=True.

@pnuu
Copy link
Member

pnuu commented Oct 9, 2024

I'll have a look at fixing the generic_image reader for at least keeping the data as float32. I ran into this while testing Geo Color and traced it to StaticImageCompositor producing float64.

@strandgren
Copy link
Collaborator Author

The next workaround to test: Create $SATPY_CONFIG_PATH/enhancements/images.yaml with

enhancements:
  default:
    operations: []

to override the default enhancement in generic.yaml only for sensor: images and then try with enhance=True.

With this I get the same corrupt output as with enhance=False, which somehow makes sense?

@pnuu
Copy link
Member

pnuu commented Oct 11, 2024

Sorry, I was unclear there. With the null enhancement use enhance=True and there shouldn't be an extra stretch happening.

@strandgren
Copy link
Collaborator Author

That's what I did. So null_enhancement + enhance=True gives the same behavior as enhance=False. But isn't that to be expected, i.e. enhancing without operations is the same as not enhancing at all?

@pnuu
Copy link
Member

pnuu commented Oct 11, 2024

Oh, silly me 🙈

How about doing a crude stretch from 0 to 255? Or 0 to 1 if that didn't work.

@pnuu
Copy link
Member

pnuu commented Oct 11, 2024

While looking into #2923 I went through the generic_image reader code. All the images with A channel are converted to floats and NaNs are placed to the no data pixels. If there's no A but the image has a nodatavals metadata item present and nodata_handling is set to "fill_value" the image is kept as integer data. I'm not sure PNG supports the no data value (or fill value).

Maybe the generic_image reader should have a feature to force a fill_value that would replace the A band whether there is a fill value in the original image or not.

@strandgren
Copy link
Collaborator Author

Oh, silly me 🙈

How about doing a crude stretch from 0 to 255? Or 0 to 1 if that didn't work.

kwargs: {stretch: 'crude', min_stretch: 0, max_stretch: 255} works! Whereas kwargs: {stretch: 'crude', min_stretch: 0, max_stretch: 1} gives the same corrupt output as with enhance=False.

But even if that workaround works, I would vote for a fix in the generic_image reader, probably something like what you suggest in #2897 (comment).

@pnuu
Copy link
Member

pnuu commented Oct 11, 2024

For some reason I can't create a new linked issue from that comment. @strandgren could you try it? Just swap the topic to Allow generic_image reader to set fill value on reading or something and I'll try to add some additional context to it afterwards.

@strandgren
Copy link
Collaborator Author

@pnuu I tried, did it work as you expected?

#2928

@pnuu
Copy link
Member

pnuu commented Oct 11, 2024

Yes, thanks! I'll add some more stuff next Monday.

@pnuu
Copy link
Member

pnuu commented Oct 18, 2024

The original float64 issue was fixed in #2923 . Lets continue with the fill value stuff in #2928 .

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

No branches or pull requests

3 participants