GDAL MiniDriver and HTTP 404

[guillaume]

When a tiled resource is configured via mini-driver, and one of the tile of the desired extent is not available (for some reason), we don't get any image back. A good enhancement would be to replace this missing content with transparent fill... Funding possible !

[Even Rouault]

I'd suggest to post your enhancement proposal on gdal-dev mailinst list where you will have perhaps a larger audience of people available for doing funded work.

[tbonfort]

The attached patch adds a <ZeroBlockHttpCodes> entry to the xml minidriver configuration file, and takes a list of HTTP return codes that lead to creating an empty image.

If the patch seems ok, I'll also add a patch to update the documentation.

[Even Rouault]

Here are my remarks :

• Is the "if( !m_parent_dataset->m_http_zeroblock_codes && (download_requests[i].nStatus == 204) ) { " really necessary ? Because in the case ZeroBlockHttpCodes is not defined you have :

      } else { 
 	331	             m_http_zeroblock_codes = ( int * ) CPLCalloc( 2, sizeof( int ) ); 
 	332	             m_http_zeroblock_codes[0] = 204; 
 	333	             m_http_zeroblock_codes[1] = 0; 
 	334	        }

So it looks like m_http_zeroblock_codes will not be NULL. Hum, perhaps if the parsing of the string fails or if the string is empty. But I think that it could default back to the above code. The motivation of this remark is that it is annoying to have the magic 204 constant in 2 places in the code.

• The parsing of the ZeroBlockHttpCodes option is overly complicated. It looks like you should just use CSLTokenizeString2( pszString, ",", CSLT_HONOURSTRINGS ) , CSLCount() and CSLDestroy().

• if(m_http_zeroblock_codes) delete m_http_zeroblock_codes; is incorrect because m_http_zeroblock_codes is allocated with CPLCalloc(). You should use CPLFree() then.

• Wondering if you shouldn't just use std::vector<int> instead of a 0 terminated C array, which is IMHO a not-so-common convention, but no strong opinion on that point. If you stick with the C array, perhaps adding a /* 0-terminated array */ in the .h would be appropriate

• You can drop <AdviseRead>false</AdviseRead> in frmts/wms/frmt_wms_swissgeo_tms.xml. This should be the default value already.

[tbonfort]

Thanks Even for the review.

Attached is an updated patch that:

• uses CSLTokenizeString2 instead of custom code for parsing the http codes
• stores the http codes in a std::vector<int>
• Added a configuration option to also call ZeroBlocks in case the server returns a ServiceException (which was actually the use-case this ticket was opened for)

[Even Rouault]

2 remarks : !strcasecmp(x,y) should be replace by EQUAL(x,y) for portability, and there's a CSLDestroy() missing to match CSLTokenizeString2().

At that point, you can add the doc for the new options and it should be ready for commit.

[tbonfort]

patch to configure which return codes lead to ZeroBlocks()

[tbonfort]

updated patch and documentation attached.

[Even Rouault]

Thomas,

I have slightly edited your patch, especially in dataset.cpp, to remove a few extra lines and fix a bug (CSLDestroy() being called twice in case of error). I believe this should work, but please check on your side.

r23192 /trunk/gdal/frmts/wms/ (dataset.cpp frmt_wms.html rasterband.cpp wmsdriver.h): WMS: add options, ZeroBlockHttpCodes and ZeroBlockOnServerException, to control which http error codes should be considered as meaning blank tile (#4169, patch by tbonfort)