SkEncodedOrigin wrong for one JPEG but not another - why?

[Webreaper]

I have two images (attached) which I'm trying to load, orient correctly, and then resize/crop to make thumbnails. My code to load the codec is super simple (I haven't included the code to resize and rotate, but can do if appropriate):

            SKCodec codec = SKCodec.Create(source.FullName);
            SKImageInfo info = codec.Info;

When I load the code via the Codec, the origin is coming back as 'Default' (TopLeft) for P8211052.JPG, which means that my resized thumbnails are not rotated correctly. However, for PA010741.JPG, the origin is correct, so my rotation code works as expected.

I'm struggling to see what the difference is between the two images; both were taken on an Olympus OMD-EM5 Mk2. If I use other image processing libs to do the same thing (e.g., the AutoOrient method in ImageSharp) it works fine - and the photos display correctly-oriented in MacOS Preview. So I'm wondering if there's either something odd about the photos, or a bug in the Codec (which seems less likely, being JPEG). Any ideas?

I could use ImageSharp - which works - but SkiaSharp can process the thumbnails about 4-6x as fast, which is important as I'm processing hundreds of thousands of images in my app.

P8211052.JPG.zip
PA010741.JPG.zip

[mattleibow]

Very interesting...

I opened both in Windows Photos app, they are upright. But, when I open them in IrfanView, one is on its side.

But what is even more interesting is that they both have the orientation, but it seems a different order...

When I open it with GIMP, the one that is upright has a popup saying that it contains EXIF metadata. But the other one does not.

I'll see what I can do. Hopefully it is just a matter of different EXIF data that maybe skia is struggling to load.

[Webreaper]

Yes, I wondered if it might be a small glitch in the exif code. Let me know what you find and if you want me to test anything!

[mattleibow]

All right. I managed to locate the issue.

What is happening is that the EXIF data is not is the usual location. As a result, there is a special entry in the tags to point to the correct location of the data. However, skia is not actually handling this case. Instead, it stops with the initial set of tags, which is just a pointer to the real data.

I'll get a fix in and then report this to the Google team so that is can reach upstream.

[Webreaper]

That's totally awesome. Thanks for the super quick response!

[mattleibow]

I have the fix building, and I have also reported it to Google: https://bugs.chromium.org/p/skia/issues/detail?id=10799

[mattleibow]

@Webreaper, I have just pushed the PR build to the preview feed. It would be great if you were able to check it out and see if it works for all your images that you can test on. I want to make sure I got all the cases. I think it should be good, but won't hurt to confirm.

Preview feed: https://aka.ms/skiasharp-eap/index.json
Exact version: 2.80.3-pr.1518.1

[Webreaper]

Will certainly give it a go - how do I use the preview feed...?
Edit: never mind, figured it out - will let you know :)

[Webreaper]

Built a new version using 2.80.3-pr.1518.1, and it's working perfectly. I see no images mis-oriented. Thanks for the quick fix!

[mattleibow]

Awesome! I'll merge them and try get a release out asap!

[ziriax]

Nice!

Would it be possible to apply this patch to 1.68.3 too?

PS: Related, I find it very unfortunate Skia doesn't provide a full blown metadata API, like Windows Imaging Component does.

[mattleibow]

I can try see if CI can build v1 again, but I am not sure if it will actually be good.

Skia is more creation than reading, so I think they decided to not put as much effort into the codecs.

[mattleibow]

I have created a CR that probably will get merged: https://skia-review.googlesource.com/c/skia/+/323217

I'm going to wait for feedback form the experts and then use whatever the final result is as this fix.

[Webreaper]

Any timescales for when a new release with this fix is going to be made?

[Webreaper]

@mattleibow any idea when this will become available in a release? It's not in the latest previews, and the 2.80.3-pr.1518.1 is gone now, so I can't pull that. Means that SkiaSharp thumbs are still broken for me 8 months after the fix was completed, and I've no way to fix them.

Could you merge this into the 2.88-preview-7 or something similar?

[mattleibow]

This should be available in the latest 2.80.3 previews. The PR releases might not stick around on the feeds for long as they are quickly merged into the main branches. In fact, I think this should be in both the 2.80.x and 2.88.x previews.

[Webreaper]

Okay, thanks. I tried 2.88-preview.7 but it failed to build on everything except Mac due to missing package:

/home/runner/work/Damselfly/Damselfly/Damselfly.Core/Damselfly.Core.csproj : error NU1102: Unable to find package SkiaSharp.NativeAssets.Linux with version (>= 2.88.0-preview.7) [/home/runner/work/Damselfly/Damselfly/Damselfly.Web/Damselfly.Web.csproj]
(full build log here)

I flipped back to 2.80.3-preview.40, but I'm still getting thumbnails where the origin/rotation isn't right - see thumb and original here, so I guess the fix isn't in that one. It's hard to tell though, with all the branches and issues you have going on right now (or it's possible this could be another bug entirely....).

[ziriax]

@mattleibow @Webreaper

I tried loading the images, and saving them back as PNG.

Here are my findings:

var bitmap = SKBitmap.Decode(inputPath); => does not automatically orient images. Related post

var image = SKImage.FromEncodedData(inputPath); => does auto-orient all images with the latest code in main, and 2.88.0-preview.36 and 2.80.3-preview.40, but doesn't work on 2.80.2 or older.

[ziriax]

@Webreaper Could you share your thumbnail generation code?

[Webreaper]

Sure. It's here: https://www.github.com/Webreaper/Damselfly/tree/master/Damselfly.Core%2FImageProcessing%2FSkiaSharpProcessor.cs

The LoadOriented... method is where the orientation action happens. Please let me know if you see anything dumb in there or better ways to do things. 😁

[Webreaper]

Okay, so after a quick test, looks like I should just change LoadOrientedBitmap(...) to use the method you suggest:

        private SKBitmap LoadOrientedBitmap( FileInfo source, int desiredWidth )
        {
            SKImage img = SKImage.FromEncodedData(source.FullName);
            var bmp = SKBitmap.FromImage(img);
            return bmp;
        }

I'm using 2.80.3-preview-40, and can confirm that with the change to the load/orientation above, it's all working now. Thanks very much! Pushed the change here: https://github.com/Webreaper/Damselfly/blob/develop/Damselfly.Core/ImageProcessing/SkiaSharpProcessor.cs#L172

PS: If you feel like reviewing my code for any other n00b errors, please feel free. ;)

[ziriax]

Your original code didn't work because you only handled 4 out of 8 possible orientations.

Your new code that used SKImage looks fine :-)

IMHO the SkiaSharp documentation should clearly state that SKBitmap does not auto orient, while SKImage does.

[Webreaper]

Thanks. May have missed that in the docs. Should be all good now - thanks for the help!

[ziriax]

Thanks. May have missed that in the docs. Should be all good now - thanks for the help!

No you didn't miss anything, the docs are missing this bit of information at first sight.

Thank you for reporting this, otherwise I would not have found this weird discrepancy between SKBitmap and SKImage!

[Webreaper]

Chaps, one follow-up on this issue.... so I've noticed some speed issues, and last night I tracked down why. The reason I was using the Codec method to load, rather than FromEncodedData is because I was using the optimisation to only load the scale closes to the resolution I need. So if I'm loading a 3,000 x 2,500 image to generate a 1,200 x 800 image, it loads closest to the 1200x800 scale - which significantly decreases the time to read the image off the disk.

The new code is taking nearly a second to load a full-res image from disk in order to resize it to 1600 on the longest side, which is really slow.

I think I took the general approach from this post: https://forums.xamarin.com/discussion/88794/fast-way-to-decode-skbitmap-from-byte-jpeg-raw - and this was why I had my own AutoOrient code (which, as you pointed out, @ziriax, was missing orientation cases).

Is there any built in load/orientation code so I can load a scaled version of the image more quickly, correctly oriented? If not, I'll have to revert to my previous code (with the additional orientation cases)..... If not, I'll snatch the code here which seems to do a better AutoOrient than the one I cribbed from this StackOverflow post.

[ziriax]

(I guess you should open a new issue, this one is closed)

No, it seems you can't do both auto-orientation and DCT-level scaling at the same time in SkiaSharp.

Skia itself supports this with its SkCodecImageGenerator, but SkiaSharp doesn't expose this.

I would suggest to open a new issue to expose this class.

However, when looking at the C++ codec of how Skia applies the orientation, we see it is just drawing the image to a canvas, creating a temporary internal bitmap! In many cases, this is not very efficient, as this could be a SIMD inplace operation...

Anyway, we can cook something like that ourselves...

Here's the code I'm using.

First, we need a way to convert the orientation into a transformation matrix, and to flip width & height if a 90 degree rotation happens.

Based on the Skia C++ code, we can make something like this:

private static (SKMatrix? transform, int width, int height) GetOrientationMatrix(SKEncodedOrigin origin, int sourceWidth, int sourceHeight)
{
	var isRotated = origin >= SKEncodedOrigin.LeftTop;
	var (w, h) = isRotated ? (sourceHeight, sourceWidth) : (sourceWidth, sourceHeight);

	// Based on SkEncodedOriginToMatrix in the Skia source code.
	SKMatrix? transform = origin switch
	{
		SKEncodedOrigin.TopRight => new SKMatrix(-1, 0, w, 0, 1, 0, 0, 0, 1),
		SKEncodedOrigin.BottomRight => new SKMatrix(-1, 0, w, 0, -1, h, 0, 0, 1),
		SKEncodedOrigin.BottomLeft => new SKMatrix(1, 0, 0, 0, -1, h, 0, 0, 1),
		SKEncodedOrigin.LeftTop => new SKMatrix(0, 1, 0, 1, 0, 0, 0, 0, 1),
		SKEncodedOrigin.RightTop => new SKMatrix(0, -1, w, 1, 0, 0, 0, 0, 1),
		SKEncodedOrigin.RightBottom => new SKMatrix(0, -1, w, -1, 0, h, 0, 0, 1),
		SKEncodedOrigin.LeftBottom => new SKMatrix(0, 1, 0, -1, 0, h, 0, 0, 1),
		_ => null
	};

	return (transform, w, h);
}

Given any SKPixMap sourceMap, we can then apply this orientation transform

var (orientationMatrix, orientedWidth, orientedHeight) = GetOrientationMatrix(orientation, sourceMap.Width, sourceMap.Height);

var transformedMap = sourceMap;

if (orientationMatrix.HasValue)
{
	// Need to apply orientation
	// Code below based on draw_orientation at https://cs.skia.org
	var newInfo = sourceMap.Info;
	newInfo.Width = orientedWidth;
	newInfo.Height = orientedHeight;

	using var tempBitmap = new SKBitmap(newInfo, SKBitmapAllocFlags.None));

	using var canvas = new SKCanvas(tempBitmap);
	using var sourceBitmap = new SKBitmap();
	// NOTE: InstallPixels *does not* transfer ownership of pixels!
	// We must keep sourceMap alive as long as sourceBitmap lives.
	// In our cause, this is just what the doctor ordered.
	sourceBitmap.InstallPixels(sourceMap);

	using var paint = new SKPaint
	{
		BlendMode = SKBlendMode.Src,
		FilterQuality = SKFilterQuality.None
	};

	canvas.SetMatrix(orientationMatrix.Value);
	canvas.DrawBitmap(sourceBitmap, 0, 0, paint);

	transformedMap = tempBitmap.PeekPixelsEx();
}

// TODO: Don't forget to dispose the transformedMap when it is different from the sourceMap!

PS: Unfortunately, in my benchmarks, the Skia solution is still twice as slow as the DirectX/WIC approach that I currently use on Windows.

PS: Somehow SkiaSharp's JPEG decoder seems to run 3x slower than should be possible based on the libjpeg-turbo. I haven't figured out why this is the case, it might be that libjpeg-turbo is incorrectly compiled or called, or that I'm just doing the benchmarking wrong. I logged this issue here #1693

[Webreaper]

Thanks, this is useful. Since it's the loading which is the lion's share of the effort, I suspect minor inefficiencies of creating a temp bitmap are less critical. For the brief benchmarking I did previously, loading scaled takes around 900ms on my M1 MBP, whereas loading the scaled version takes around 150ms. So it's quite a saving. :)

I'll raise the issue you mentioned later today when I get a chance.