Bitmap.Save writes a raw RGB array as BGR

[MV10]

I am wondering if the firmware bug described here is the cause:

https://www.raspberrypi.org/forums/viewtopic.php?t=262579

I'm writing a raw file like this:

var pathname = ramdiskPath + "snapshot.raw";
File.Delete(pathname);
var cam = GetConfiguredCamera();
MMALCameraConfig.Encoding = MMALEncoding.RGB24;
MMALCameraConfig.EncodingSubFormat = MMALEncoding.RGB24;
cam.ConfigureCameraSettings();
using var handler = new ImageStreamCaptureHandler(pathname);
await cam.TakeRawPicture(handler);
cam.Cleanup();

Then I run it through Bitmap.Save to output a JPEG, and as we discussed, the B/R channels are reversed:

// use the 1296x972 resolution's padded buffer dimensions of 1312x976
var width = 1312;
var height = 976;
var pixelFormat = PixelFormat.Format24bppRgb;

var data = await File.ReadAllBytesAsync(ramdiskPath + "snapshot.raw");

using var bitmap = new Bitmap(width, height, pixelFormat);
BitmapData bmpData = null;
try
{
    bmpData = bitmap.LockBits(new Rectangle(0, 0, width, height), ImageLockMode.WriteOnly, pixelFormat);
    var ptr = bmpData.Scan0;
    int size = bmpData.Stride * height;
    Marshal.Copy(data, 0, ptr, size);
}
finally
{
    bitmap.UnlockBits(bmpData);
}

bitmap.Save(ramdiskPath + "snapshot.jpg", ImageFormat.Jpeg);

[MV10]

Interesting... my wife has a PhotoShop subscription, and it's able to open the raw file with the correct colors, so maybe Bitmap.Save is the problem after all. I can't find anywhere in PS that it'll show whether it's RGB or BGR (the mode is RGB3 but BGR isn't listed).

[techyian]

Yes, I'm seeing a few articles which suggest Bitmap.Save does indeed store as BGR and a conversion will need applying to swap the B and R values around. Will be interesting to see how quick we can get the mem swap for a full frame, and slightly frustrating we'd need to do this in the first place considering we're telling Bitmap what pixel format to expect.

[MV10]

That's frustrating, I'm doing some searches and seeing the same. Too bad we can't use the GPU. In the PR where this surfaced you mentioned running it through an image encoder ... I'm thinking this is probably the fastest fix (being hardware based) but I'm wondering if/how it can be done internally so that the library consumer doesn't have to worry about it.

I guess I could also hack in a fix to the parallel processing loops to swap R/B based on a flag (based on the raw setting, I think). Bizarre, though, that an RGB format is interpreted as BGR. Makes you wonder what the boys in Redmond were smoking.

[MV10]

I ran some tests. Since we know this is only needed when calling Bitmap.Save from a raw format, presently this only affects convolutions. The good news is that I can't detect any significant impact from an extra pass in each cell to swap those bytes (which is quite surprising).

I tested 3x3 convolutions with the same resolution in my earlier convolution perf tests -- 1296 x 972 (which produces a 1312 x 976 buffer ... just 64 more padding pixels for the entire image). As before, the processing loop with no BGR swap ranged from 305ms to 359ms, averaging somewhere around 325ms. (That's just the analysis/process loop, not saving the bitmap.)

Adding a second pass for the channel swap maybe added just a few milliseconds to the processing operation. I actually saw one faster run at 304ms and one slower at 362ms but the average seemed a bit higher.

It has to be a completely separate pass over the data (that is, before or after the convolution effects are calculated), but that buffer defaults to a cell size of 41 x 61 pixels which is pretty small. Basically the byte-swap is a three-step operation, so 2500 array reads and 5000 array writes.

I actually expected it to be a lot worse. Even though it isn't caused by the PR, it affects the PR so I'll push the fix and I think we can wrap this up. We'll have to keep it in mind for anything in the future which does Bitmap.Save, of course. Indeed, this could be a useful general-purpose implementation of the pixel-level delegate idea I mentioned in #185 (although it would always be much faster to handle it internally for library-provided effects).

I'll fix the issue title for posterity, but if you agree, I think we can close this out.