vips_magickload behaves differently from vips_magickload_buffer when dealing with NEF files

[lkempf]

Bug report

Describe the bug
When loading a NEF file vips_magickload produces a image of the correct dimension while vips_magickload_buffer produces a image of dimension 160x120 (probably some embedded thumbnail). This small image is also produced when loading the image as a TIFF (which is what libvips identifies the image as).

To Reproduce
Steps to reproduce the behavior:

1. Use Image https://www.mannyphoto.com/D700D3/_DSC4175.NEF
2. Execute the following in python

import pyvips
img_bytes = open('./_DSC4175.NEF', 'rb').read()
img1 = pyvips.Image.magickload('./_DSC4175.NEF')
img2 = pyvips.Image.magickload_buffer(img_bytes)
print(img1.width, img2.width)

3. Output is 4284 160 not the expected 4284 4284.

Expected behavior
Both methods should produce the same image.

Actual behavior
Loading from buffer produces a much smaller image.

Environment

• OS: Arch Linux
• Vips: vips-8.14.1

[kleisauke]

ImageMagick doesn't have a buffer recognizer for Nikon's NEF file format. Since this file format uses the standard TIFF header, it tries to load the buffer through its TIFF loader. libvips has a special path to workaround this limitation of *magick, but that does not include support for NEF.

I had a quick attempt to add a NEF sniffer for this, see e.g. commit kleisauke@ab5d0e3, but somehow(?) this would cause a regression on the file-based loader, as it outputs 160 4284 on the above reproducer.

[lkempf]

That's not the output of the file based loader, that's the width for both loaders. Sorry for the confusing output format.

Edit: Sorry I misunderstood your comment. That's weird.

[kleisauke]

this would cause a regression on the file-based loader, as it outputs 160 4284 on the above reproducer.

I just reported this upstream at ImageMagick/ImageMagick#6242.

[kleisauke]

I just opened PR #3460 for this.

[uhthomas]

I can see that #3460 did not actually include a fix for this @kleisauke. Do you have any ideas on how we can reliably differentiate between NEF and TIFF files?

[kleisauke]

@uhthomas AFAIK, the only reliable way is to get parse the make TIFF tag and check for NIKON CORPORATION or just NIKON. See for example how it's handled in RawSpeed:
https://github.com/darktable-org/rawspeed/blob/75bfffedf5378a17136da18d1560d7e0c3509338/src/librawspeed/decoders/NefDecoder.cpp#L63

This would also mean that the magick sniffer needs to inspect more than 256 bytes, which would probably affect performance.

Looking at the linked issues, I think the issue is that tiffload has a higher priority than magickload.

libvips/libvips/foreign/tiffload.c

Lines 181 to 183 in 8d5f331

	/* We are fast, but must test after openslideload.
	*/
	foreign_class->priority = 50;

libvips/libvips/foreign/magick7load.c

Lines 365 to 368 in 8d5f331

	/* We need to be well to the back of the queue since vips's
	* dedicated loaders are usually preferable.
	*/
	foreign_class->priority = -100;

One way to fix this is to prevent TIFF-based RAW images from being loaded via libtiff, see e.g. commit kleisauke@a061854 (this feels quite hackish 😅).

A dedicated LibRaw/RawSpeed loader would make more sense, please subscribe to #2075 for updates regarding this.

[jcupitt]

Looking at the linked issues, I think the issue is that tiffload has a higher priority than magickload.

Yes, the libvips TIFF loader is quite a bit quicker than the imagemagick one, so it needs to be higher priority. As you say, the best solution is probably adding a libraw loader to libvips. Someone just neds to do the work heh.

[mertalev]

We fixed the issue for now by compiling libvips without libtiff. Adding libraw support would be amazing, but the make tag solution sounds great too. I would prefer a minor performance hit if it meant a reliably correct output, though I obviously can't speak for other libvips users.