[gdal-dev] RE: progressive rendering

[Norman Barker]

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