[gdal-dev] RE: progressive rendering

Wed Aug 27 18:08:39 EDT 2008

Even,

Thanks for the example, that has made it clearer, particularly the mention of a queue, I like that - also you mention we can avoid the lockbuffer / unlock at the api level which was my other worry.

Give me a day or so to update the RFC with both approaches - pattern vs pattern :-). I can see that a queue would work with GDAL, and an observer (typical java pattern) would work with the swig java wrapper - I am sure there is a way to accommodate both; perhaps queue within the format driver and a simulated subscriber within the swig wrapper.

Many thanks for your comments, I will post again when the RFC is updated.

Norman
-----Original Message-----
From: Even Rouault [mailto:even.rouault at mines-paris.org] 
Sent: Wednesday, August 27, 2008 4:00 PM
To: Norman Barker
Cc: gdal-dev at lists.osgeo.org
Subject: Re: [gdal-dev] RE: progressive rendering

Norman,

In fact, Adam's proposal reminds me a lot of the asynchronous queue object from GLIB-2.0

See : http://library.gnome.org/devel/glib/2.12/glib-Asynchronous-Queues.html

I don't know JPIP, but I can image that the driver would start a thread when
AsyncRasterIO() is called. It communicates with the server and receives the updates with a polling loop. When it has received an update,it put the received data as well as the parameters describing the window, etc... in a structure (let's call it a ticket), pushes that ticket in a stack and goes on pushing tickets, or wait for the ticket to be consumed by the reader (both are possible, even if you can't push continuously new tickets as memory will increase, so the working thread would have to go in idle mode until the queue decreases a bit)

The NextAsyncRasterIOMessage() call will check that some message is available and unstack the first ticket. In fact, the LockBuffer() / UnlockBuffer() could probably be avoided at the API level. Of course the implementation of
NextAsyncRasterIOMessage() needs an internal mutex to protect the accesses to the queue.

Really, that would match very closely the GAsyncQueue*.

Design pattern against design pattern....

Le Wednesday 27 August 2008 23:39:44, vous avez écrit :
> Even,
>
> I really am not sure how the interface from Adam would work with JPIP 
> - I need to see an example of this working.  Sure I understand how the 
> interface he proposes could work with the occassional update, but here 
> we are talking many updates a second and adding the locking, blocking 
> at the user level etc. I think will be a big perform issues and 
> additional complexity for the user to take advantage of streaming data services.
>
> So from the proposed new design
>
> while (...) {
>    GDALAsyncRasterIOMessage *m = ds->NextAsyncRasterIOMessage(true);
>
>    if (m) {
>      if (m->what == GARM_UPDATE) {
> // lock the buffer so there will be no updates while we read from it
>        m->asyncrasterio->LockBuffer( /* ... */ );
>
> // display updated region
>
>        m->asyncrasterio->UnlockBuffer();
>      } else {
> // handle completion, display error message, ...
>
>      }
>
>      ds->ReleaseAsyncRasterIOMessage(m);
>    }
>
> }
>
> In my opinion this should be hidden from the user, but with the new 
> strategy would have to be implemented.
>
> I am open to a redesign, but I definitely need to see a pattern 
> reference or an example of this working to be convinced.
>
> To me the benefits of using an observer like pattern is the simplicity 
> for the user to use this library, they just add a listener and get updates.
>
> http://www.codeproject.com/KB/architecture/observer.aspx
>
>
> Norman
>
> -----Original Message-----
> From: Even Rouault [mailto:even.rouault at mines-paris.org]
> Sent: Wednesday, August 27, 2008 3:27 PM
> To: Norman Barker
> Cc: gdal-dev at lists.osgeo.org
> Subject: Re: [gdal-dev] RE: progressive rendering
>
> Norman,
>
> I've seen your answers to my question on the wiki.
>
> So, if the call is not blocking, it means that the callback will not 
> be called back in the thread of the caller, but by a thread created by 
> the driver. That might be a problem, as many bad things can happen 
> then if the dataset is called before the last call from the driver is 
> made, or if the user_data or the "objects" that it contains are 
> destroyed, etc. OK, this is not the fault of the driver, but it is 
> very easy for users to write bad code in such a design. Or they need 
> to have an obvious way of knowing that it is definitely finished. I 
> must also mention that you can't for example use safely the same 
> GDALDataset* object from different threads without locks.
>
> Your API proposal could be OK if the RasterIO() call is blocking and 
> the callback called from the caller thread. But I think that if we 
> really want a non-blocking API, something looking like Adam Nowacki's 
> proposal is rather to be considered.
>
> I think that a callback called from another thread is going to be a 
> source of lot of problems.
>
> Le Wednesday 27 August 2008 22:42:04 Even Rouault, vous avez écrit :
> > Another question :
> >
> > how will that work with the block cache mechanism ?
> >
> > If GDALDataset::RasterIO() is overloaded by the driver, the block 
> > cache will not be used.
> > Not sure if it is a problem, but that might be interesting to think 
> > about that. I've the feeling that this is a bit linked to how the 
> > modified
> > RasterIO() on the dataset works with the RasterIO() on the band.
> >
> > Le Wednesday 27 August 2008 22:34:58 Even Rouault, vous avez écrit :
> > > Norman,
> > >
> > > as I see you are currently editing your proposal and I've not yet 
> > > made my comments, here I go.
> > >
> > > I would like that the dataset object to be added as the first 
> > > argument of the callback, and a void* user_data to be added as the 
> > > last argument
> > >
> > > So the call would be :
> > >
> > > ds->RasterIO (GF_Read, xOff, yOff, xSize, ySize, NULL (1), 
> > > ds->bufXSize,
> > > bufYSize, bufType, nBandCount, bandMap, nPixelSpace, nLineSpace, 
> > > nBandSpace, pfnProgressIO, pProgressUserData)
> > >
> > > typedef (*GDALRasterIOProgressFunc) (GDALDatasetH hDS, int xOff, 
> > > int yOff, int xSize, int ySize, void * pBuf, int bufXSize, int 
> > > bufYSize, GDALDataType bufType, int nBandCount, int* bandMap, int 
> > > nPixelSpace, int nLineSpace, int nBandSpace, void* 
> > > pProgressUserData)
> > >
> > >
> > > I've a few questions so that your explanations and proposals are 
> > > clear to my mind.
> > >
> > > - Is the extended version of the RasterIO() call still blocking as 
> > > the current version?
> > > My understanding of the discussion is "yes", but I would like a 
> > > yes/no confirmation. If "no", then I don't understand at all how it can work.
> > >
> > > - What happens if the user specifies a not NULL argument as the 
> > > output buffer ( in (1) ) ? What happens if the user specifies GF_Write ?
> > > It is probably an argument for a name change, something like 
> > > ProgressiveRasterIO.
> > >
> > > - Is it guaranteed that the bufType, nBandCount, bandMap, 
> > > nPixelSpace, nLineSpace, nBandSpace specified by the caller will 
> > > be the values passed to the call back function ?
> > >
> > > - Maybe it can make sense to add some way of cancelling the whole 
> > > RasterIO call by providing a callback, like the standard progress 
> > > callback (GDALProgressFunc in gdal.h) mechanism do ? Because the
> > > RasterIO() will spend most of the time waiting for data. It could 
> > > resume from time to time to call that callback and see if the user 
> > > still wants the request to be continued. It would be nice if the 
> > > mechanism could provide some percentage of the total progress as 
> > > it might be tedious for the user to compute that ? But that's 
> > > probably not easy to define if you first update the whole request 
> > > area with a low resolution, and then at higher resolutions.
> > >
> > > - What happens if the user issues another call to RasterIO(), 
> > > traditionnal version and/or your extended version, in the 
> > > pfnProgressIO callback ? Is it forbidden, or does the answer 
> > > depend of the underlying driver ?
> > >
> > > Best regards
> > >
> > > _______________________________________________
> > > gdal-dev mailing list
> > > gdal-dev at lists.osgeo.org
> > > http://lists.osgeo.org/mailman/listinfo/gdal-dev
> >
> > _______________________________________________
> > gdal-dev mailing list
> > gdal-dev at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/gdal-dev