[gdal-dev] GDAL Progressive Data Support RFC Discussion

[Even Rouault]

Frank,

The API & concepts look good to me. I also agree with Tamas' remarks.

I think what Tamas means by synchronized wait approach is something based on 
pthread_cond_new() / pthread_cond_signal() / pthread_cond_wait() for Unix or 
CreateEvent() / SetEvent() / WaitForSingleObject() for Windows, which enables 
a thread to wait for an event signaled by another thread. But this is just an 
implementation detail of the JPIPKAK driver.

As far as the API is concerned, here are my questions :

* I've skimmed the past discussions on RFC24 and I see one of your remarks 
was "The GDALAsyncRasterIO object should be renamed GDALAsyncRasterReader or 
something that does not imply that writing is supported unless that is 
intended for the future". Perhaps that we can leave the name of 
GDALAsyncRasterIO/BeginAsyncRasterIO() but introduce a GDALRWFlag eRWFlag 
parameter as first parameter of BeginAsyncRasterIO() if we want to allow 
future use for writing ?

* LockBuffer() / UnlockBuffer(): the RFC says "The LockBuffer() and 
UnlockBuffer() methods can be used to temporarily disable updates to the 
buffer while application code accesses the buffer". Shouldn't it say 
rather "... *should* be used ... " as the user has no way to tell if the 
driver implementation makes it necessary or not to use 
LockBuffer()/UnlockBuffer(). 

* Shouldn't LockBuffer() have a return code instead of void ? In case the 
underlying locking primitive fail. Shouldn't LockBuffer() have a timeout 
parameter for consistency ? Or we want it to wait for infinite time ?

* The timeout parameter of GetNextUpdatedRegion(). This is related to the 
meaning of the timeout parameter of the related CPL primitive 
CPLAcquireMutex(). 
	-  I notice that CPLAcquireMutex() use a dfWaitInSeconds parameter : wouldn't 
it be more consistant to use it also for GetNextUpdatedRegion() ?
	- Shouldn't we have a special constant meaning infinite waiting, instead of 
using arbirtrary numbers such as 100.0 used in 
JPIPKAKAsyncRasterIO::GetNextUpdatedRegion().

* BeginAsyncRasterIO() : neither the RFC nor the doc of the method explains if 
it is legal or not to initiate multiple AsyncRasterIO() without waiting the 
first ones to be termintated before beginning a new one. My understanding of 
the JPIPKAK driver is that this particular driver allows 2 concurrent 
AsyncRasterIO(). Perhaps just adding a note in the doc of 
BeginAsyncRasterIO() to say that this is a driver specific behaviour might be 
sufficient. In which case, the doc should probably add "@return The 
GDALAsyncRasterIO object representing the request or NULL if the request 
cannot be started". (by the way, it might fail for other reasons such as 
wrong parameter values)

Best regards,

Even

Le Friday 12 March 2010 02:09:15 Frank Warmerdam, vous avez écrit :
> Folks,
>
> I have spent several days working on the JPIP driver initiated by
> Norman Barker, and also worked on by Garrett Briggs.  This driver
> uses a proposed new method for asynchronous and progressive data
> access for GDAL.  The driver now seems to be working fairly well, and
> based on that work I have fleshed out the RFC for the proposed new
> API.
>
> The implementation can be found in:
>
>    http://svn.osgeo.org/gdal/sandbox/normanb/gdal
>
> It is somewhat hard to build due to the Kakadu dependency - some makefile
> hacking may be required.
>
> The RFC can be found at:
>
>    http://trac.osgeo.org/gdal/wiki/rfc24_progressive_data_support
>
> Currently I am calling for discussion on the proposed interface - similar
> to what it was on the last round, though I've tried to clarify a few issues
> and streamlined away a few items.
>
> Depending on feedback I hope to call for a vote on the RFC within a couple
> of days.
>
> Best regards,