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

Use ColorInterp or MaskFlags to check alpha band #51

Merged
merged 2 commits into from Aug 14, 2018

Conversation

kprabowo
Copy link
Contributor

@kprabowo kprabowo commented Aug 2, 2018

I got this error: https://github.com/mapbox/rasterio/blob/master/rasterio/_warp.pyx#L907

and this code explains that it checks alpha band by GDAL color interpretation.
https://github.com/mapbox/rasterio/blob/master/rasterio/_warp.pyx#L898

p.s: sorry for my bad english

@vincentsarago
Copy link
Member

Hi @kn-id, thanks for opening this PR.
To be honest I'm not sure if ColorInterp.alpha == colorinterp: is better than MaskFlags.alpha in flags to check for internal alpha band.

While testing the code I couldn't find a dataset that was passing MaskFlags.alpha in flags but failing ColorInterp.alpha == colorinterp:.

Do you mind sharing your image ? or at least the output of rio info ?

Thanks

@vincentsarago
Copy link
Member

also seems @mojodna is using a slightly different test in marblecutter which might be worth to steal copy ;-) https://github.com/mojodna/marblecutter/blob/c7170aa063f5003a6d1acbc95753831d23d5b776/marblecutter/__init__.py#L264-L267

@vincentsarago
Copy link
Member

worth noting that

for colorinterp in src.colorinterp:
    if ColorInterp.alpha == colorinterp:
        return True

won't work but should be

if ColorInterp.alpha in src.colorinterp:
    return True

I'm happy to update the function to something like:

def has_alpha_band(src):
    """Check for alpha band or mask in source."""
    if (
        any([MaskFlags.alpha in flags for flags in src.mask_flag_enums])
        or ColorInterp.alpha in src.colorinterp
    ):
        return True
    return False

but I'd love to make sure we understand why the first test (MaskFlags.alpha) didn't work for your image.

@kprabowo
Copy link
Contributor Author

Hi @vincentsarago, thanks for replying. Here's the output of rio info from my raster image:

{
    "blockxsize": 256,
    "blockysize": 256,
    "bounds": [
        412620.45813000004,
        9667517.052310001,
        414303.85761000006,
        9669577.69789
    ],
    "colorinterp": [
        "red",
        "green",
        "blue",
        "alpha"
    ],
    "compress": "lzw",
    "count": 4,
    "crs": "EPSG:32751",
    "descriptions": [
        null,
        null,
        null,
        null
    ],
    "driver": "GTiff",
    "dtype": "uint8",
    "height": 14721,
    "indexes": [
        1,
        2,
        3,
        4
    ],
    "interleave": "pixel",
    "lnglat": [
        122.22127123720861,
        -2.998450073465773
    ],
    "mask_flags": [
        [
            "nodata"
        ],
        [
            "nodata"
        ],
        [
            "nodata"
        ],
        [
            "nodata"
        ]
    ],
    "nodata": null,
    "res": [
        0.13998000000000002,
        0.13998000000000002
    ],
    "shape": [
        14721,
        12026
    ],
    "tiled": true,
    "transform": [
        0.13998000000000002,
        0.0,
        412620.45813000004,
        0.0,
        -0.13998000000000002,
        9669577.69789,
        0.0,
        0.0,
        1.0
    ],
    "units": [
        null,
        null,
        null,
        null
    ],
    "width": 12026
}

And here's the link to my image https://drive.google.com/file/d/1pJdJ6sdNUQLrOPn4AZqaHoSN_zJrFihS/view?usp=sharing

@vincentsarago
Copy link
Member

having mask_flags=Nodata and colorinterp=alpha is kind of bizarre. Any idea how the file was created ?

@kprabowo
Copy link
Contributor Author

It was created using pix4d. Any idea how to solve this?
should i update the code according to your suggestion?

def has_alpha_band(src):
    """Check for alpha band or mask in source."""
    if (
        any([MaskFlags.alpha in flags for flags in src.mask_flag_enums])
        or ColorInterp.alpha in src.colorinterp
    ):
        return True
    return False

@vincentsarago
Copy link
Member

pix4d its really doing something weird, we already talked about it here
rasterio/rasterio#1373 (comment)

I have no objection to update the code with the proposed change
#51 (comment) but just noting that having the nodata value ignore in the file is kind of unwanted, this should be reported to pix4d.

@kprabowo kprabowo changed the title Use ColorInterp instead of MaskFlags to check alpha band Use ColorInterp and MaskFlags to check alpha band Aug 14, 2018
@kprabowo
Copy link
Contributor Author

Hi @vincentsarago, the code has been updated, please review this. Thank you.

@kprabowo kprabowo changed the title Use ColorInterp and MaskFlags to check alpha band Use ColorInterp or MaskFlags to check alpha band Aug 14, 2018
Copy link
Member

@vincentsarago vincentsarago left a comment

Choose a reason for hiding this comment

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

👍 thanks @kn-id

@vincentsarago vincentsarago merged commit a0dbe56 into cogeotiff:master Aug 14, 2018
@vincentsarago
Copy link
Member

@kn-id I'll add a test case for this new code and publish a new version of rio-tiler later today

@vincentsarago
Copy link
Member

Hi @kn-id, I cannot create a file to test the new code (to add the test fixture). I tried to use gdal_translate In.tif out.tif -outsize 3% 3% to get a < 1Mb file, but the file doesn't behave like the initial file from pix4d.
Would you be able to create a small file directly from pix4d ?

@kprabowo
Copy link
Contributor Author

@vincentsarago , I'm only able to get as small as 1,5 Mb. Here's the link https://drive.google.com/open?id=0Bzo9qLZh8_--c0RsS2l0WnBzbW1ya2RWZFlZNjFCZzZRUTRR
Is this okay?

@vincentsarago
Copy link
Member

thanks @kn-id, just release rio-tiler==1.0rc2 with the fix 🙏

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.

None yet

2 participants