[gdal-dev] RE: progressive rendering

[Even Rouault]

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), 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