pytroll/satpy

generic_image reader returns data as float64 for PNG images

Closed this issue ยท 19 comments

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

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

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

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.

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 commented

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.

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 commented

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 commented

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.

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 commented

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

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 commented

Oh, silly me ๐Ÿ™ˆ

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

pnuu commented

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.

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 commented

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.

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

#2928

pnuu commented

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

pnuu commented

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