Opened 6 years ago

Closed 5 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

Description

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 5 years ago.
patch to configure which return codes lead to ZeroBlocks?()

Download all attachments as: .zip

Change History (9)

comment:1 Changed 6 years ago by 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.

comment:2 Changed 6 years ago by tbonfort

Cc: tbonfort added

comment:3 Changed 6 years ago by 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.

comment:4 Changed 6 years ago by Even Rouault

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 Changed 5 years ago by 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)

comment:6 Changed 5 years ago by 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.

Changed 5 years ago by tbonfort

Attachment: 4169.patch added

patch to configure which return codes lead to ZeroBlocks?()

comment:7 Changed 5 years ago by tbonfort

updated patch and documentation attached.

comment:8 Changed 5 years ago by Even Rouault

Milestone: 1.9.0
Resolution: fixed
Status: newclosed

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)

Note: See TracTickets for help on using tickets.