ARG Driver: byte order should never change

[dzwarg]

The byte order of .ARG files is fixed in the spec, and should not vary. Remove checks for CPL_LSB, and always set bNative to TRUE

[Even Rouault]

The current code looks correct to me. It enforces that the on-file order is LSB.

#ifdef CPL_LSB

int bNative = TRUE;

#else

int bNative = FALSE;

#endif

If your machine has LSB ordering, the bNative = TRUE, and indeed the on-file order is your native CPU order. If your machine has MSB ordering, bNative = FALSE and RawRasterBand() will do the byte-swapping

Am I missing something ?

[dzwarg]

The .arg file should always have the same byte order, so no swapping should be happening. According to the spec (​https://github.com/azavea/geotrellis/wiki/ARG-Specification):

"All data is stored in network byte order (big-endian)"

[Even Rouault]

Ok, then you need to invert the values :

#ifdef CPL_LSB
    int	bNative = FALSE;
#else
    int bNative = TRUE;
#endif

The #ifdef CPL_LSB test is true if the CPU is little-endian. We definitely need different values for bNative if the CPU is little-endian or big-endian. If I understand well, they are just inverted right now.

[Even Rouault]

Your patch will not do what you want. On Intel CPUs, it will assume that the data is stored in little-endian order on the disk, which contradicts with "All data is stored in network byte order (big-endian)"

[dzwarg]

Better fix for reading/writing properly ordered bytes.

[dzwarg]

The problem here is two-fold:

1. ARGDataset had the flags reversed for bNative during open.
2. ARGDataset read data straight from the source dataset into the raw destination '.arg' file, bypassing any proper handling.

The fixes to the above are:

1. Reverse the flags during open.
2. Use the RasterIO method on the RasterBands to read from the source dataset, and write to the destination dataset. These RasterIO calls respect the byte order, whereas the raw write with VSIFWriteL did not.

The patch 'networkorder.patch' replaces the earlier bogus patch with the correct behavior, modeled after the mffdataset

This patch also addressses #4780

[Even Rouault]

Your patch looks saner, although I suspect there's a bug in the RasterIO() call (both the read and write). The second argument should be, I think, nXBlock * nXBlockSize, instead of just nXBlock. You wouldn't notice the difference with a scanline oriented input file, but you could likely see the bug if you use for example a tiled geotiff as input of CreateCopy()

[dzwarg]

Thanks! Will update the patch with this fix.

[dzwarg]

Fix for blocking in X and Y; addition of tests to verify bug & fix

[dzwarg]

Fixed in r24795