[Gdal-dev] C# Bindings

[Simon Perkins]

Tamas Szekeres wrote:
> I've added the base implementation with the following signature into 
> the CVS:
>
> public int ReadRasterInternal(int xOff, int yOff, int xSize, int
> ySize, IntPtr buffer, int buf_xSize, int buf_ySize, int buf_type, int
> pixelSpace, int lineSpace);
>
> public int WriteRasterInternal(int xOff, int yOff, int xSize, int
> ySize, IntPtr buffer, int buf_xSize, int buf_ySize, int buf_type, int
> pixelSpace, int lineSpace);

That seems fine. Though I guess it feels slightly odd to me for a public 
function to be called "Internal". How about just calling these functions 
ReadRaster() and WriteRaster(), same as the more type safe functions, 
but just using a different set of arguments?

> And tested the code with the following sample:

And it worked? Hmm, I notice that you use Format24bppRgb when creating 
the Bitmap, and then Format32bppRgb when locking it. I'm surprised that 
didn't throw an exception. Also the pixelspace of 4 is appropriate for 
32bpp, but not for 24bpp. Perhaps windows uses 4 bytes per pixel even 
when you specify 24bpp?

>
> At this point I would avoid creating /unsafe code into the interface
> so I will leave the user alone with the pointers and the fixed
> statements.
> However I would add the specializations depending on the array types
> using Marshal.Copy
> between the managed and the unmanaged memory.

So how does this work? i.e. where does the unmanaged buffer that 
GDALRasterIO initially reads into get allocated? In C code somewhere? Or 
is there some way of getting an IntPtr pointing to an allocated buffer 
in C#? And then you copy it into a C# array in some CS code? And then 
switch back to C to free the unmanaged memory? And you can do all this 
without using the unsafe keyword? Guess it'll be easier to visualize it 
when you've written it.

FWIW, my personal feeling is that writing code like this that doesn't 
use the unsafe keyword makes you feel better, but when you look more 
closely it's a pretty dubious benefit. At the end of the day, you're 
passing pointers to buffers into unmanaged C code, and so really, that 
SHOULD be declared as unsafe. That C code could do all kinds of nasty 
things to your C# data structures, outside of your control. The fact 
that you can get around it by using IntPtr seems to me to be more a 
failing of the language, than a genuinely "safe" way to do things. 
Unless I'm missing something.

Anyway, I'm still not especially keen on any extra copying of buffers. 
As long as the direct interface using IntPtr is available, I guess users 
can always choose to write their own more efficient methods to read GDAL 
data into arrays, using fixed() { }. But still, seems a shame to build 
in inefficiency just to avoid using the unsafe keyword, when in my 
opinion, the code should not be considered "safe" anyway.



> How about supporting ReadBlock/WriteBlock? Would it be required?

I think this is a pretty low priority. Most C++ apps use RasterIO() 
directly, which in turn makes calls to ReadBlock() and WriteBlock() 
(actually IReadBlock() and IWriteBlock()), which are implemented by the 
author of the particular GDAL driver being used. If people really want 
to read and write blocks, then can always get the block width and height 
using GDALGetBlockXSize(), and so forth, and then use RasterIO() with 
windows of that size.

>
> According to your questions:
>
>> - For any functions that use an array as a buffer, it would be
>> convenient to add a bufferOffset int argument that says at what point in
>> the buffer reading/writing should begin. This allows people to use GDAL
>> to read into arrays that contain pixels in "packed" format. This only
>> needs to appear in the overload of these functions that has the
>> pixelSpace and lineSpace args, since packed format buffers require these
>> to be specified as well.
>
> Would you describe an expected signature and a test sample for this
> issue? At which side of the interface should we implement this? I
> guess at the C side it would be easier.

The signature would be something like:

  int ReadRaster(int xOff, int yOff, int xSize, int ySize, byte[] buf, 
int bufXSize, int bufYSize,
                 int bufOffset, int pixelSpace, int lineSpace);

Test code (untested!):

           // Get the GDAL Band objects from the Dataset
           Band redBand = ds.GetRasterBand(1);
           Band greenBand = ds.GetRasterBand(2);
           Band blueBand = ds.GetRasterBand(3);

           // Get the width and height of the Dataset
           int width = ds.RasterXSize;
           int height = ds.RasterYSize;

           // Create a byte buffer to store the pixel data in "packed" 
format
           byte[] buffer = new byte[width * height * 4];

           // Use GDAL raster reading methods to read the image data 
directly into the buffer
           // We use the bufferOffset argument to begin reading at 
different start indices in
           // the buffer.
           blueBand.ReadRasterInternal(0, 0, width, height, buffer, 
width, height, 1, 0, 4, width);
           greenBand.ReadRasterInternal(0, 0, width, height, buffer, 
width, height, 1, 1, 4, width);
           redBand.ReadRasterInternal(0, 0, width, height, buffer, 
width, height, 1, 2, 4, width);

           // Create a Bitmap to store the GDAL image in
           Bitmap bitmap = new Bitmap(width, height, 
PixelFormat.Format32bppRgb);

           // Copy the data from the packed buffer into Bitmap
           BitmapData bitmapData = bitmap.LockBits(new Rectangle(0, 0, 
width, height), ImageLockMode.ReadWrite,
                PixelFormat.Format32bppRgb);
           try {
             Marshal.Copy(buffer, bitmapData.Scan0, 0, width * height * 4);
           }
           finally {
             bitmap.UnlockBits(bitmapData);
           }

           bitmap.Save("Data/myfile.bmp");


Thinking about it some more, I think even for the IntPtr overload of 
ReadRaster / WriteRaster, it would make sense to add the bufferOffset 
argument, whenever we use the pixelSpace and lineSpace arguments. This 
avoids having to use that "new IntPtr(buf.ToInt32() + 1)" stuff. All 
three args are only really used when dealing with packed pixel format 
buffers.

For convenience, it would be nice to have overloads that assume default 
values for the last three parameters (bufferOffset, pixelSpace and 
lineSpace) for the common case where we're not dealing with packed 
buffers. In these cases, bufferOffset = 0, pixelSpace = 
sizeof(datatype), and lineSpace = bufXSize * pixelSpace.


Cheers,

Sy