Improve LibRaw thumbnail choice

Make use of LibRaw 0.21.0's extended thumbnail API.

fiv-thumbnail.c

@@ -219,9 +219,11 @@ adjust_thumbnail(cairo_surface_t *thumbnail, double row_height)
 }
 
 static cairo_surface_t *
-orient_thumbnail(cairo_surface_t *surface, FivIoOrientation orientation)
+orient_thumbnail(cairo_surface_t *surface)
 {
-	if (!surface || orientation <= FivIoOrientation0)
+	int orientation = (intptr_t) cairo_surface_get_user_data(
+		surface, &fiv_io_key_orientation);
+	if (orientation <= FivIoOrientation0)
 		return surface;
 
 	double w = 0, h = 0;
@@ -240,6 +242,249 @@ orient_thumbnail(cairo_surface_t *surface, FivIoOrientation orientation)
 	return oriented;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+#ifdef HAVE_LIBRAW
+#if LIBRAW_VERSION >= LIBRAW_MAKE_VERSION(0, 21, 0)
+
+static int
+extract_libraw_compare(const void *a, const void *b)
+{
+	const libraw_thumbnail_item_t **t1 = (const libraw_thumbnail_item_t **) a;
+	const libraw_thumbnail_item_t **t2 = (const libraw_thumbnail_item_t **) b;
+	float p1 = (float) (*t1)->twidth * (*t1)->theight;
+	float p2 = (float) (*t2)->twidth * (*t2)->theight;
+	return (p2 < p1) - (p1 < p2);
+}
+
+static gboolean
+extract_libraw_unpack(libraw_data_t *iprc, int *flip, GError **error)
+{
+	int count = iprc->thumbs_list.thumbcount;
+	if (count <= 0) {
+		set_error(error, "no thumbnails found");
+		return FALSE;
+	}
+
+	// The old libraw_unpack_thumb() goes for the largest thumbnail,
+	// but we currently want the smallest usable thumbnail. Order them.
+	libraw_thumbnail_item_t **sorted = g_malloc_n(count, sizeof *sorted);
+	for (int i = 0; i < count; i++)
+		sorted[i] = &iprc->thumbs_list.thumblist[i];
+	qsort(sorted, count, sizeof *sorted, extract_libraw_compare);
+
+	// With the raw.pixls.us database, zero dimensions occur in two cases:
+	//  - when thumbcount should really be 0,
+	//  - with the last, huge JPEG thumbnail in CR3 raws.
+	// The maintainer refuses to change anything about it (#589).
+	int i = 0;
+	while (i < count && (!sorted[i]->twidth || !sorted[i]->theight))
+		i++;
+
+	// Ignore thumbnails whose decoding is likely to be a waste of time.
+	// XXX: This primarily targets the TIFF/EP shortcut code,
+	// because decoding a thumbnail will always be /much/ quicker than a render.
+	// TODO(p): Maybe don't mark raw image thumbnails as low-quality
+	// if they're the right aspect ratio, and of sufficiently large size.
+	// And I still worry about tflip.
+	float output_pixels = (float) iprc->sizes.iwidth * iprc->sizes.iheight;
+	// Note that the ratio may even be larger than 1, as seen with CR2 files.
+	while (i < count &&
+		(float) sorted[count - 1]->twidth * sorted[count - 1]->theight >
+			output_pixels * 0.75)
+		count--;
+
+	// The smallest size thumbnail is very often forced to be 4:3,
+	// and the remaining space is filled with black, looking quite wrong.
+	// It isn't really possible to strip those borders, because many are JPEGs.
+	//
+	// Another reason to skip thumbnails of mismatching aspect ratios is
+	// to avoid browser items from jumping around when low-quality thumbnails
+	// get replaced with their final versions.
+	//
+	// Note that some of them actually have borders on all four sides
+	// (Nikon/D50/DSC_5155.NEF, Nikon/D70/20170902_0047.NEF,
+	// Nikon/D70s/RAW_NIKON_D70S.NEF), or even on just one side
+	// (Leica/LEICA M MONOCHROM (Typ 246), Leica/M (Typ 240)).
+	// Another interesting possibility is Sony/DSC-HX99/DSC00001.ARW,
+	// where the correct-ratio thumbnail has borders but the main image doesn't.
+	//
+	// The problematic thumbnail is usually, but not always, sized 160x120,
+	// and some of them may actually be fine.
+	float output_ratio = (float) iprc->sizes.iwidth / iprc->sizes.iheight;
+	while (i < count) {
+		// XXX: tflip is less reliable than libraw_dcraw_make_mem_thumb()
+		// and reading out Orientation from the resulting Exif.
+		float ratio = sorted[i]->tflip == 5 || sorted[i]->tflip == 6
+			? (float) sorted[i]->theight / sorted[i]->twidth
+			: (float) sorted[i]->twidth / sorted[i]->theight;
+		if (fabsf(ratio - output_ratio) < 0.05)
+			break;
+		i++;
+	}
+
+	// Avoid pink-tinted readouts of CR2 IFD2 (#590).
+	//
+	// This thumbnail can also have a black stripe on the left and the top,
+	// which we should remove if using fixed LibRaw > 0.21.1.
+	if (i < count && iprc->idata.maker_index == LIBRAW_CAMERAMAKER_Canon &&
+		sorted[i]->tformat == LIBRAW_INTERNAL_THUMBNAIL_KODAK_THUMB)
+		i++;
+
+	if (i < count)
+		i = sorted[i] - iprc->thumbs_list.thumblist;
+
+	g_free(sorted);
+	if (i == count) {
+		set_error(error, "no suitable thumbnails found");
+		return FALSE;
+	}
+
+	int err = 0;
+	if ((err = libraw_unpack_thumb_ex(iprc, i))) {
+		set_error(error, libraw_strerror(err));
+		return FALSE;
+	}
+	*flip = iprc->thumbs_list.thumblist[i].tflip;
+	return TRUE;
+}
+
+#else  // LIBRAW_VERSION < LIBRAW_MAKE_VERSION(0, 21, 0)
+
+static gboolean
+extract_libraw_unpack(libraw_data_t *iprc, int *flip, GError **error)
+{
+	int err = 0;
+	if ((err = libraw_unpack_thumb(iprc))) {
+		set_error(error, libraw_strerror(err));
+		return FALSE;
+	}
+
+	// The main image's "flip" often matches up, but sometimes doesn't, e.g.:
+	//  - Phase One/H 25/H25_Outdoor_.IIQ
+	//  - Phase One/H 25/H25_IT8.7-2_Card.TIF
+	//  - Leaf/Aptus 22/L_003172.mos (JPEG)
+	*flip = iprc->sizes.flip
+	return TRUE;
+}
+
+#endif  // LIBRAW_VERSION < LIBRAW_MAKE_VERSION(0, 21, 0)
+
+// LibRaw does a weird permutation here, so follow the documentation,
+// which assumes that mirrored orientations never happen.
+static FivIoOrientation
+extract_libraw_unflip(int flip)
+{
+	switch (flip) {
+	break; case 0:
+		return FivIoOrientation0;
+	break; case 3:
+		return FivIoOrientation180;
+	break; case 5:
+		return FivIoOrientation270;
+	break; case 6:
+		return FivIoOrientation90;
+	break; default:
+		return FivIoOrientationUnknown;
+	}
+}
+
+static cairo_surface_t *
+extract_libraw_bitmap(libraw_processed_image_t *image, int flip, GError **error)
+{
+	// Anything else is extremely rare.
+	if (image->colors != 3 || image->bits != 8) {
+		set_error(error, "unsupported bitmap thumbnail");
+		return NULL;
+	}
+
+	cairo_surface_t *surface = cairo_image_surface_create(
+		CAIRO_FORMAT_RGB24, image->width, image->height);
+	guint32 *out = (guint32 *) cairo_image_surface_get_data(surface);
+	const unsigned char *in = image->data;
+	for (guint64 i = 0; i < image->width * image->height; in += 3)
+		out[i++] = in[0] << 16 | in[1] << 8 | in[2];
+	cairo_surface_mark_dirty(surface);
+
+	FivIoOrientation orient = extract_libraw_unflip(flip);
+	cairo_surface_set_user_data(
+		surface, &fiv_io_key_orientation, (void *) (intptr_t) orient, NULL);
+	return surface;
+}
+
+static cairo_surface_t *
+extract_libraw(GFile *target, GMappedFile *mf, GError **error)
+{
+	cairo_surface_t *surface = NULL;
+	libraw_data_t *iprc = libraw_init(
+		LIBRAW_OPIONS_NO_MEMERR_CALLBACK | LIBRAW_OPIONS_NO_DATAERR_CALLBACK);
+	if (!iprc) {
+		set_error(error, "failed to obtain a LibRaw handle");
+		return NULL;
+	}
+
+	int err = 0;
+	if ((err = libraw_open_buffer(iprc, (void *) g_mapped_file_get_contents(mf),
+			 g_mapped_file_get_length(mf)))) {
+		set_error(error, libraw_strerror(err));
+		goto fail;
+	}
+	if ((err = libraw_adjust_sizes_info_only(iprc))) {
+		set_error(error, libraw_strerror(err));
+		goto fail;
+	}
+
+	int flip = 0;
+	if (!extract_libraw_unpack(iprc, &flip, error))
+		goto fail;
+
+	libraw_processed_image_t *image = libraw_dcraw_make_mem_thumb(iprc, &err);
+	if (!image) {
+		set_error(error, libraw_strerror(err));
+		goto fail;
+	}
+
+	// Bitmap thumbnails generally need rotating, e.g.:
+	//  - Hasselblad/H4D-50/2-9-2017_street_0012.fff
+	//  - OnePlus/One/IMG_20150729_201116.dng (and more DNGs in general)
+	//
+	// JPEG thumbnails generally have the right rotation in their Exif, e.g.:
+	//  - Canon/EOS-1Ds Mark II/RAW_CANON_1DSM2.CR2
+	//  - Leica/C (Typ 112)/Leica_-_C_(Typ_112)-_3:2.RWL
+	//  - Nikon/1 S2/RAW_NIKON_1S2.NEF
+	//  - Panasonic/DMC-FZ18/RAW_PANASONIC_LUMIX_FZ18.RAW
+	//  - Panasonic/DMC-FZ70/P1000836.RW2
+	//  - Samsung/NX200/2013-05-08-194524__sam6589.srw
+	//  - Sony/DSC-HX95/DSC00018.ARW
+	// Note that LibRaw inserts its own Exif segment if it doesn't find one,
+	// and this may differ from flip.
+	//
+	// Some files are problematic and we won't bother with special-casing:
+	//  - Nokia/Lumia 1020/RAW_NOKIA_LUMIA_1020.DNG (bitmap) has wrong color.
+	//  - Ricoh/GXR/R0017428.DNG (JPEG) seems to be plainly invalid.
+	switch (image->type) {
+		gboolean dummy;
+	case LIBRAW_IMAGE_JPEG:
+		surface = render(
+			target, g_bytes_new(image->data, image->data_size), &dummy, error);
+		break;
+	case LIBRAW_IMAGE_BITMAP:
+		surface = extract_libraw_bitmap(image, flip, error);
+		break;
+	default:
+		set_error(error, "unsupported embedded thumbnail");
+	}
+
+	libraw_dcraw_clear_mem(image);
+fail:
+	libraw_close(iprc);
+	return surface;
+}
+
+#endif  // HAVE_LIBRAW
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
 cairo_surface_t *
 fiv_thumbnail_extract(GFile *target, FivThumbnailSize max_size, GError **error)
 {
@@ -253,139 +498,25 @@ fiv_thumbnail_extract(GFile *target, FivThumbnailSize max_size, GError **error)
 	if (!mf)
 		return NULL;
 
-	// Bitmap thumbnails generally need rotating, e.g.:
-	//  - Hasselblad/H4D-50/2-9-2017_street_0012.fff
-	//  - OnePlus/One/IMG_20150729_201116.dng (and more DNGs in general)
-	// Though it's apparent LibRaw doesn't adjust the thumbnails to match
-	// the main image's "flip" field (it just happens to match up often), e.g.:
-	//  - Phase One/H 25/H25_Outdoor_.IIQ (correct Orientation in IFD0)
-	//  - Phase One/H 25/H25_IT8.7-2_Card.TIF (correctly missing in IFD0)
-	//
-	// JPEG thumbnails generally have the right rotation in their Exif, e.g.:
-	//  - Canon/EOS-1Ds Mark II/RAW_CANON_1DSM2.CR2
-	//  - Leica/C (Typ 112)/Leica_-_C_(Typ_112)-_3:2.RWL
-	//  - Nikon/1 S2/RAW_NIKON_1S2.NEF
-	//  - Panasonic/DMC-FZ18/RAW_PANASONIC_LUMIX_FZ18.RAW
-	//  - Panasonic/DMC-FZ70/P1000836.RW2
-	//  - Samsung/NX200/2013-05-08-194524__sam6589.srw
-	//  - Sony/DSC-HX95/DSC00018.ARW
-	//
-	// Some files are problematic and we won't bother with special-casing:
-	//  - Leaf/Aptus 22/L_003172.mos (JPEG)'s thumbnail wrongly contains
-	//    Exif Orientation 6, and sizes.flip also contains 6.
-	//  - Nokia/Lumia 1020/RAW_NOKIA_LUMIA_1020.DNG (bitmap) has wrong color.
-	//  - Ricoh/GXR/R0017428.DNG (JPEG) seems to be plainly invalid.
-	FivIoOrientation orientation = FivIoOrientationUnknown;
-	cairo_surface_t *surface = NULL;
-#ifndef HAVE_LIBRAW
-	// TODO(p): Implement our own thumbnail extractors.
-	set_error(error, "unsupported file");
-#else  // HAVE_LIBRAW
 	// In this case, g_mapped_file_get_contents() returns NULL, causing issues.
 	if (!g_mapped_file_get_length(mf)) {
 		set_error(error, "empty file");
-		goto fail;
+		return NULL;
 	}
 
-	libraw_data_t *iprc = libraw_init(
-		LIBRAW_OPIONS_NO_MEMERR_CALLBACK | LIBRAW_OPIONS_NO_DATAERR_CALLBACK);
-	if (!iprc) {
-		set_error(error, "failed to obtain a LibRaw handle");
-		goto fail;
-	}
-
-	int err = 0;
-	if ((err = libraw_open_buffer(iprc, (void *) g_mapped_file_get_contents(mf),
-			 g_mapped_file_get_length(mf)))) {
-		set_error(error, libraw_strerror(err));
-		goto fail_libraw;
-	}
-
-#if LIBRAW_VERSION >= LIBRAW_MAKE_VERSION(0, 21, 0)
-	if (!iprc->thumbs_list.thumbcount) {
-		set_error(error, "no thumbnails found");
-		goto fail_libraw;
-	}
-
-	// The old libraw_unpack_thumb() goes for the largest thumbnail,
-	// but we currently want the smallest thumbnail.
-	// TODO(p): To handle the ugly IFD0 thumbnail of NEF,
-	// try to go for the second smallest size. Remember to reflect tflip.
-	int best_index = 0;
-	float best_pixels = INFINITY;
-	for (int i = 0; i < iprc->thumbs_list.thumbcount; i++) {
-		float pixels = (float) iprc->thumbs_list.thumblist[i].twidth *
-			(float) iprc->thumbs_list.thumblist[i].theight;
-		if (pixels && pixels < best_pixels) {
-			best_index = i;
-			best_pixels = pixels;
-		}
-	}
-	if ((err = libraw_unpack_thumb_ex(iprc, best_index))) {
-		set_error(error, libraw_strerror(err));
-		goto fail_libraw;
-	}
-#else
-	if ((err = libraw_unpack_thumb(iprc))) {
-		set_error(error, libraw_strerror(err));
-		goto fail_libraw;
-	}
-#endif
-
-	libraw_processed_image_t *image = libraw_dcraw_make_mem_thumb(iprc, &err);
-	if (!image) {
-		set_error(error, libraw_strerror(err));
-		goto fail_libraw;
-	}
-
-	gboolean dummy = FALSE;
-	switch (image->type) {
-	case LIBRAW_IMAGE_JPEG:
-		surface = render(
-			target, g_bytes_new(image->data, image->data_size), &dummy, error);
-		orientation = (int) (intptr_t) cairo_surface_get_user_data(
-			surface, &fiv_io_key_orientation);
-		break;
-	case LIBRAW_IMAGE_BITMAP:
-		// Anything else is extremely rare.
-		if (image->colors != 3 || image->bits != 8) {
-			set_error(error, "unsupported bitmap thumbnail");
-			break;
-		}
-
-		surface = cairo_image_surface_create(
-			CAIRO_FORMAT_RGB24, image->width, image->height);
-		guint32 *out = (guint32 *) cairo_image_surface_get_data(surface);
-		const unsigned char *in = image->data;
-		for (guint64 i = 0; i < image->width * image->height; in += 3)
-			out[i++] = in[0] << 16 | in[1] << 8 | in[2];
-		cairo_surface_mark_dirty(surface);
-
-		// LibRaw actually turns an 8 to 5, so follow the documentation.
-		switch (iprc->sizes.flip) {
-		break; case 3: orientation = FivIoOrientation180;
-		break; case 5: orientation = FivIoOrientation270;
-		break; case 6: orientation = FivIoOrientation90;
-		}
-		break;
-	default:
-		set_error(error, "unsupported embedded thumbnail");
-	}
-
-	libraw_dcraw_clear_mem(image);
-fail_libraw:
-	libraw_close(iprc);
-#endif  // HAVE_LIBRAW
-
-fail:
+	cairo_surface_t *surface = NULL;
+#ifdef HAVE_LIBRAW
+	surface = extract_libraw(target, mf, error);
+#else  // ! HAVE_LIBRAW
+	// TODO(p): Implement our own thumbnail extractors.
+	set_error(error, "unsupported file");
+#endif  // ! HAVE_LIBRAW
 	g_mapped_file_unref(mf);
 
-	// This hardcodes Exif orientation before adjust_thumbnail() might do so,
-	// before the early return below.
-	surface = orient_thumbnail(surface, orientation);
-	if (!surface || max_size < FIV_THUMBNAIL_SIZE_MIN ||
-		max_size > FIV_THUMBNAIL_SIZE_MAX)
-		return surface;
+	if (!surface)
+		return NULL;
+	if (max_size < FIV_THUMBNAIL_SIZE_MIN || max_size > FIV_THUMBNAIL_SIZE_MAX)
+		return orient_thumbnail(surface);
 
 	cairo_surface_t *result =
 		adjust_thumbnail(surface, fiv_thumbnail_sizes[max_size].size);

meson.build

@@ -165,12 +165,6 @@ exe = executable('fiv', 'fiv.c', 'fiv-view.c', 'fiv-io.c', 'fiv-context-menu.c',
 	dependencies : dependencies,
 	win_subsystem : 'windows',
 )
-if gdkpixbuf.found()
-	executable('io-benchmark', 'fiv-io-benchmark.c', 'fiv-io.c',
-		'xdg.c', tiff_tables,
-		build_by_default : false,
-		dependencies : [dependencies, gdkpixbuf])
-endif
 
 desktops += 'fiv-jpegcrop.desktop'
 jpegcrop = executable('fiv-jpegcrop', 'fiv-jpegcrop.c', rc, config,
@@ -194,6 +188,11 @@ if get_option('tools').enabled()
 			dependencies : tools_dependencies,
 			c_args: tools_c_args)
 	endforeach
+
+	if gdkpixbuf.found()
+		executable('benchmark-io', 'tools/benchmark-io.c', 'fiv-io.c', 'xdg.c',
+			tiff_tables, dependencies : [dependencies, gdkpixbuf])
+	endif
 endif
 
 gsettings_schemas = ['fiv.gschema.xml']

tools/benchmark-io.c

@@ -1,7 +1,7 @@
 //
-// fiv-io-benchmark.c: see if we suck
+// benchmark-io.c: measure and compare image loading times
 //
-// Copyright (c) 2021 - 2022, Přemysl Eric Janouch <p@janouch.name>
+// Copyright (c) 2021 - 2023, Přemysl Eric Janouch <p@janouch.name>
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted.
@@ -32,35 +32,36 @@ timestamp(void)
 static void
 one_file(const char *filename)
 {
-	double since_us = timestamp();
+	GFile *file = g_file_new_for_commandline_arg(filename);
+	double since_us = timestamp(), us = 0;
 	FivIoOpenContext ctx = {
-		.uri = g_filename_to_uri(filename, NULL, NULL),
+		.uri = g_file_get_uri(file),
 		.screen_dpi = 96,
 		// Only using this array as a redirect.
 		.warnings = g_ptr_array_new_with_free_func(g_free),
 	};
 
 	cairo_surface_t *loaded_by_us = fiv_io_open(&ctx, NULL);
+	g_clear_object(&file);
 	g_free((char *) ctx.uri);
 	g_ptr_array_free(ctx.warnings, TRUE);
 	if (!loaded_by_us)
 		return;
 
 	cairo_surface_destroy(loaded_by_us);
-	double us = timestamp() - since_us;
+	us = timestamp() - since_us;
 
-	double since_pixbuf = timestamp();
+	double since_pixbuf = timestamp(), pixbuf = 0;
 	GdkPixbuf *gdk_pixbuf = gdk_pixbuf_new_from_file(filename, NULL);
-	if (!gdk_pixbuf)
-		return;
+	if (gdk_pixbuf) {
+		cairo_surface_t *loaded_by_pixbuf =
+			gdk_cairo_surface_create_from_pixbuf(gdk_pixbuf, 1, NULL);
+		g_object_unref(gdk_pixbuf);
+		cairo_surface_destroy(loaded_by_pixbuf);
+		pixbuf = timestamp() - since_pixbuf;
+	}
 
-	cairo_surface_t *loaded_by_pixbuf =
-		gdk_cairo_surface_create_from_pixbuf(gdk_pixbuf, 1, NULL);
-	g_object_unref(gdk_pixbuf);
-	cairo_surface_destroy(loaded_by_pixbuf);
-	double pixbuf = timestamp() - since_pixbuf;
-
-	printf("%f\t%f\t%.0f%%\t%s\n", us, pixbuf, us / pixbuf * 100, filename);
+	printf("%.3f\t%.3f\t%.0f%%\t%s\n", us, pixbuf, us / pixbuf * 100, filename);
 }
 
 int

tools/info.h

@@ -247,19 +247,23 @@ parse_exif_ifd(struct tiffer *T, const struct tiff_entry *info)
 	}
 
 	// This is how Exif specifies it, which doesn't follow TIFF 6.0.
-	if (info == tiff_entries && compression == TIFF_Compression_JPEG &&
+	// Also support CR2 IFD1, which isn't tagged with compression at all.
+	if (info == tiff_entries && /* compression == TIFF_Compression_JPEG && */
 		jpeg > 0 && jpeg_length > 0 &&
-		jpeg + jpeg_length < (T->end - T->begin)) {
+		jpeg + jpeg_length <= (T->end - T->begin)) {
 		ifd = jv_set(ifd, jv_string("JPEG image data"),
 			parse_jpeg(
 				jv_object(), T->begin + jpeg, jpeg_length));
 	}
 
+	// As specified by DRAFT TIFF Technical Note 2 + TIFFphotoshop.pdf.
 	// Theoretically, there may be more strips, but this is not expected.
+	// Also support CR2 IFD0, which is tagged with the "wrong" compression.
 	if (info == tiff_entries &&
-		compression == TIFF_Compression_JPEGDatastream &&
+		(compression == TIFF_Compression_JPEGDatastream ||
+			compression == TIFF_Compression_JPEG) &&
 		strip_offsets > 0 && strip_byte_counts > 0 &&
-		strip_offsets + strip_byte_counts < (T->end - T->begin)) {
+		strip_offsets + strip_byte_counts <= (T->end - T->begin)) {
 		ifd = jv_set(ifd, jv_string("JPEG image data"),
 			parse_jpeg(
 				jv_object(), T->begin + strip_offsets, strip_byte_counts));