rio cogeo create fails when converting sentinel-2 preview files about rio-cogeo HOT 16 CLOSED

@alando46 sure thing

I'll try to fix this, this week

from rio-cogeo.

@DanSchoppe @alando46 @mojodna could you please review the changes in #82 please

from rio-cogeo.

@alando46 thanks for opening the issue.
basically you are trying to create a COG with a default 512x512 blocksize : https://github.com/cogeotiff/rio-cogeo/blob/master/rio_cogeo/profiles.py#L95-L96

You can overwrite this by passing --co BLOCKXSIZE=128 --co BLOCKYSIZE=128 (or even 192)

from rio-cogeo.

thanks @vincentsarago I must have missed this in the docs. after chatting with @mojodna, it seems like there is an opportunity for an optimization here: it might be useful if rio-cogeo could detect when the default block size is larger than the tiled source image and then default to use the image dimensions as blocksize, or maybe as in your example, the next lowest blocksize which is a power of 2.

from rio-cogeo.

There is one difference between GDAL and rasterio. With GDAL you can create a file with blocksize that are bigger than the actual files, while this is not possible with rasterio. I'm going to check rasterio code to understand this better.

To be honest the simplest solution will be to adjust the blocksize but I'm not a big fan on taking decision for the user (meaning changing blocksize when raster is too small).

from rio-cogeo.

Hmm interesting that they diverge on blocksize handling.

For us, I was able to resolve this by passing the --co BLOCKXSIZE=128 --co BLOCKYSIZE=128 args you suggested above. In terms of rasterio, I imagine you have a much better sense of if such a default configuration would be useful for the greater community.

Does rasterio ever include configuration tips in errors? For example instead of :
ValueError: blockxsize exceeds raster width.
Would it make sense to adjust this to:
ValueError: blockxsize exceeds raster width. Set smaller output blocksize with --co BLOCKXSIZE={value} --co BLOCKYSIZE={value}

In any case, now we have an issue with a fix that others might land on if they encounter the same situation. Thanks @vincentsarago for thinking through this.

from rio-cogeo.

@alando46
so here is where it happens in rasterio https://github.com/mapbox/rasterio/blob/4f28d081579d877723bd806e1947686d908513db/rasterio/_io.pyx#L1090-L1094

Would it make sense to adjust this to:
ValueError: blockxsize exceeds raster width. Set smaller output blocksize with --co BLOCKXSIZE={value} --co BLOCKYSIZE={value}

--co option is specific to rio-cogeo implementation, I don't think the error message from rasterio could be better.
I'm still not sure why rasterio and GDAL behave differently but I'm going to ask on https://rasterio.groups.io/g/main/topics

from rio-cogeo.

I'm also bumping into this issue for small GeoTIFFs. A couple thoughts / questions:

• If @vincentsarago finds out there's good reason for rasterio to disallow block sizes > image dimensions, it would be nice if rio-cogeo could handle this gracefully by automatically reducing the block size
• Is there a limitation on valid blocksizes? Does it have to be a factor of two? I tried to set blockysize to 221 and got this error:

_TIFFVSetField:/vsimem/c4f5dd7c-af38-4620-908a-7e5604006991.: Bad value 221 for "TileLength" tag

Update: After some testing, it seems as though the blocksize needs to be a multiple of 16 🤔

from rio-cogeo.

I'm sorry @DanSchoppe and @alando46,

I think minimum tile size is 64px and should be one of 64, 128, 256, 512, 1024, 2048

To be honest maybe a better solution is not to try to create COG for files < 256px, but that should be on your application.

from rio-cogeo.

@vincentsarago Thanks for the information.

I'm assuming there's not a way to access tiles from a tiny raster if it doesn't get COG'd? If that's the case, I'll have to take your advice and stop supporting rasters < 256px x 256px.

from rio-cogeo.

I'm assuming there's not a way to access tiles from a tiny raster if it doesn't get COG'd? If that's the case, I'll have to take your advice and stop supporting rasters < 256px x 256px.

Well for small raster (< 256px x 256px) The COG specification doesn't apply because they can't be tiled, thus we don't need to run rio-cogeo but only a simple rasterio.shutil.copy to create a GeoTIFF)

from rio-cogeo.

I'm seeking a common workflow to prepare rasters for rio-tiler access, even if they're very small rasters. In the very small case, I was assuming rio-cogeo would essentially produce a COG of a single tile. But alas, this doesn't seem to be the case.

Thanks for the rasterio.shutil.copy() suggestion, @vincentsarago. Would a raster produced using this method be compatible with rio-tiler?

Alternatively, perhaps an application could be tasked with padding the input raster prior to calling rio-cogeo? Checking out rasterio.pad() now.

from rio-cogeo.

Would a raster produced using this method be compatible with rio-tiler?

rio-tiler can read any raster data (any driver / any size) so it shouldn't be a problem

here is how you app could look:

from rio_cogeo.cogeo import cog_translate

import rasterio
from rasterio.shutil import copy

config = dict(NUM_THREADS=100, GDAL_TIFF_OVR_BLOCKSIZE=128)

output_profile = {
    "driver": "GTiff",
    "interleave": "pixel",
    "tiled": True,
    "blockxsize": 256,
    "blockysize": 256,
    "compress": "DEFLATE",
}

with rasterio.open(src_path) as src_dst:
    w, h = src_dst.width, src_dst.height

if w < 256 or h < 256:
    output_profile = {
        "driver": "GTiff",
        "interleave": "pixel",
        "compress": "DEFLATE",
    }
   copy(src_path, dst_path, **dst_kwargs)
else:
    cog_translate(
        src_path,
        cog_path,
        output_profile,
        in_memory=False,
        config=config,
        quiet=True,
    )

from rio-cogeo.

@vincentsarago why not include that logic in the CLI? If the goal is to be a fire and forget tool, it should handle the edge cases.

As it stands now, users either need to write a custom wrapper for the CLI and/or check the size of their inputs in whatever's calling it (not entirely trivial, and easy to forget) in order to conditionally change the tile size if the source is too small.

from rio-cogeo.

Thanks, @vincentsarago for the tip!

I've started down the path you suggested of using shutil for small rasters. This has some unfortunate side-effects:

• This is incompatible with other ecosystem tooling. For instance, I cannot use rio-glui to inspect the result.
• I'll have to manually recreate some of the functionality I've enjoyed from rio-cogeo for the "small raster" case:
  • RGBA input -> RGB + internal_bitmask
  • rasterio config (looks like this is accomplished in rio-cogeo with with rasterio.Env(**config):)
  • specifying desired band indexes

Given these limitations, I think I'll again look into simply padding the input raster so it can share the rio-cogeo / COG pipeline.

from rio-cogeo.

Thanks @DanSchoppe, yeah I didn't think to much but I see why it could be valuable and @mojodna had a good point too.

If someone wants to start a PR to add checks and warning somewhere after https://github.com/cogeotiff/rio-cogeo/blob/master/rio_cogeo/cogeo.py#L108 I'll happilly support and review.

from rio-cogeo.