Opened 11 years ago

Closed 11 years ago

#4169 closed enhancement (fixed)

GDAL MiniDriver and HTTP 404

Reported by: guillaume Owned by: warmerdam
Priority: normal Milestone: 1.9.0
Component: default Version: svn-trunk
Severity: normal Keywords: mini-driver
Cc: tbonfort, nowakpl


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 !

Attachments (1)

4169.patch (8.3 KB ) - added by tbonfort 11 years ago.
patch to configure which return codes lead to ZeroBlocks()

Download all attachments as: .zip

Change History (9)

comment:1 by Even Rouault, 11 years ago

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.

comment:2 by tbonfort, 11 years ago

Cc: tbonfort added

comment:3 by tbonfort, 11 years ago

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.

comment:4 by Even Rouault, 11 years ago

Cc: nowakpl added

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.

comment:5 by tbonfort, 11 years ago

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)

comment:6 by Even Rouault, 11 years ago

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.

by tbonfort, 11 years ago

Attachment: 4169.patch added

patch to configure which return codes lead to ZeroBlocks()

comment:7 by tbonfort, 11 years ago

updated patch and documentation attached.

comment:8 by Even Rouault, 11 years ago

Milestone: 1.9.0
Resolution: fixed
Status: newclosed


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)

Note: See TracTickets for help on using tickets.