Frank,<br><br>The described approach with regards to the API seems reasonable for me, I recall I took part in the conversation that time when the initial version have been worked out, and I remember of most aspect.<br><br>
Just by having a quick review of the code I have the following questions/concerns:<br><br><br>1. The implementation of the PERSISTENT flag is handled by using a static curl handle shared by multiple threads without having synchronization in the cpl code. I'm not sure whether it's completely guarded by the driver level mutex (pGlobalMutex), however it would be better to do the sync in-place where the shared variable is declared. This would require for example to place http_persistent_handle either at the driver, or do the sync in the CPL code.<br>
<br>2. Not sure whether it's safe to use the same curl handle by multiple threads (either with syncing or without).<br><br>3. Could you explain the reason of using 2 worker threads (with different priorities) at the driver? I'm not sure about the use case where these 2 threads are required to have.<br>
<br>4. It would be reasonable to use (probably by extenting the CPL code) sychronized waiting instead of <a href="http://en.wikipedia.org/wiki/Busy_waiting">busy-waiting</a> implemented in the diver code. This cause for example the implementation of JPIPKAKAsyncRasterIO::Stop() getting odd, and error prone. For instance it may result in a wait forever condition if the thread would terminate without setting the exit flag correctly. By using a synchronized wait approach instead (ie an event handle) the wait function would terminate with an error if the owner thread have been terminated in the meantime.<br>
<br>5. The diver doesn't seem to do any specific if CPLAcquireMutex fails or returns with timeout (by not getting the ownership of the mutex in effect)<br><br><br>By all means I have no problem with the API (which appears to be the primary subject of this RFC)<br>
<br>Best regards,<br><br>Tamas<br><br><br><div style="visibility: hidden; display: inline;" id="avg_ls_inline_popup"></div><style type="text/css">#avg_ls_inline_popup { position:absolute; z-index:9999; padding: 0px 0px; margin-left: 0px; margin-top: 0px; width: 240px; overflow: hidden; word-wrap: break-word; color: black; font-size: 10px; text-align: left; line-height: 13px;}</style><br>
<div class="gmail_quote">2010/3/12 Frank Warmerdam <span dir="ltr"><<a href="mailto:warmerdam@pobox.com">warmerdam@pobox.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Folks,<br>
<br>
I have spent several days working on the JPIP driver initiated by<br>
Norman Barker, and also worked on by Garrett Briggs. This driver<br>
uses a proposed new method for asynchronous and progressive data<br>
access for GDAL. The driver now seems to be working fairly well, and<br>
based on that work I have fleshed out the RFC for the proposed new<br>
API.<br>
<br>
The implementation can be found in:<br>
<br>
<a href="http://svn.osgeo.org/gdal/sandbox/normanb/gdal" target="_blank">http://svn.osgeo.org/gdal/sandbox/normanb/gdal</a><br>
<br>
It is somewhat hard to build due to the Kakadu dependency - some makefile<br>
hacking may be required.<br>
<br>
The RFC can be found at:<br>
<br>
<a href="http://trac.osgeo.org/gdal/wiki/rfc24_progressive_data_support" target="_blank">http://trac.osgeo.org/gdal/wiki/rfc24_progressive_data_support</a><br>
<br>
Currently I am calling for discussion on the proposed interface - similar<br>
to what it was on the last round, though I've tried to clarify a few issues<br>
and streamlined away a few items.<br>
<br>
Depending on feedback I hope to call for a vote on the RFC within a couple<br>
of days.<br>
<br>
Best regards,<br>
-- <br>
---------------------------------------+--------------------------------------<br>
I set the clouds in motion - turn up | Frank Warmerdam, <a href="mailto:warmerdam@pobox.com" target="_blank">warmerdam@pobox.com</a><br>
light and sound - activate the windows | <a href="http://pobox.com/%7Ewarmerdam" target="_blank">http://pobox.com/~warmerdam</a><br>
and watch the world go round - Rush | Geospatial Programmer for Rent<br>
<br>
_______________________________________________<br>
gdal-dev mailing list<br>
<a href="mailto:gdal-dev@lists.osgeo.org" target="_blank">gdal-dev@lists.osgeo.org</a><br>
<a href="http://lists.osgeo.org/mailman/listinfo/gdal-dev" target="_blank">http://lists.osgeo.org/mailman/listinfo/gdal-dev</a><br>
</blockquote></div><br>