GNOME Bugzilla – Bug 701622
[PATCH] Add JPEG image density metadata support
Last modified: 2014-10-22 14:12:45 UTC
Add JPEG image density metadata support. Useful for tools like simple scan which know the density.
Created attachment 246043 [details] [review] Add JPEG image density metadata support
Created attachment 246045 [details] [review] Add JPEG image density metadata support Fixed whitespace
Created attachment 246046 [details] [review] Add JPEG image density metadata support Correctly set density information after structure is reset
Created attachment 246119 [details] [review] Add JPEG image density metadata support Added bug link into comment (last revision, I promise :)
Created attachment 246192 [details] [review] Add JPEG image density metadata support Corrected grammar in error thrown if density-unit not valid
Created attachment 246229 [details] [review] Add JPEG image density metadata support Document density settings
Hmm, do we really have to deal with units here ? I think x_dpi=300 y_dpi=300 would be all that could ever be needed ?
I just matched the JPEG format so it would always work. In terms of what I need for simple-scan, DPI is fine. If we only support DPI it means on writing a JPEG we always set the units to dots-per-inch, and on reading we convert the units if they are dots-per-cm (with a slight loss in precision). I'm happy to change the patch if that meets the gdk-pixbuf convention (ease of use over accuracy).
Hi Matthias! There's related bug: https://bugzilla.gnome.org/show_bug.cgi?id=466372 I'm voting for this issue too.
(In reply to comment #8) > I just matched the JPEG format so it would always work. In terms of what I need > for simple-scan, DPI is fine. > > If we only support DPI it means on writing a JPEG we always set the units to > dots-per-inch, and on reading we convert the units if they are dots-per-cm > (with a slight loss in precision). > > I'm happy to change the patch if that meets the gdk-pixbuf convention (ease of > use over accuracy). How big would the change of precision be? Do we usually (or ever) get odd numbers for the image resolution (say, not multiples of 300 for data coming from scanners)?
Created attachment 289070 [details] [review] Add JPEG image density metadata support I don't have any data about what real world cases there are. In theory, a scanner might give you an image that is done at 800 dots per cm. This would be rounded to 315dpi. The maximum error would be 0.5dpi I guess. Here's the patch updated to convert to/from dpi if you prefer that.
*** Bug 498721 has been marked as a duplicate of this bug. ***
Review of attachment 289070 [details] [review]: You also need to update gdk_pixbuf_get_option() to mention those same new "read" options. Nearly there! ::: gdk-pixbuf/io-jpeg.c @@ +520,3 @@ gint i; char otag_str[5]; + char *density_str; I wouldn't even try to align them, or I would align letters with letters (not with the asterisk). @@ +603,3 @@ + switch (cinfo.density_unit) + { "{" on the same line as the switch. @@ +615,3 @@ + case 2: + /* Dots per cm - convert into dpi */ + density_str = g_strdup_printf ("%d", (int) round (cinfo.X_density / 2.54)); Can you make that a macro? @@ +1300,3 @@ + x_density = -1; + + if (x_density < 0 || I think we should fail if a density is passed in and comes out as zero, no? @@ +1321,3 @@ + y_density = -1; + + if (y_density < 0 || Ditto
Created attachment 289127 [details] [review] Add JPEG image density metadata support Fix whitespace, disallow x-dpi=0 / y-dpi=0. Add macro for converting dots per cm to dpi
Created attachment 289128 [details] [review] Add JPEG image density metadata support Add documentation to gdk_pixbuf_get_option
Review of attachment 289128 [details] [review]: Looks good otherwise to commit to master. ::: gdk-pixbuf/gdk-pixbuf.c @@ +899,3 @@ * the "multipage" option string to "yes" when a multi-page TIFF is loaded. + * Since 2.32 the JPEG loader sets "x-dpi" and "y-dpi" if the file contains + * image density information. "in dots-per-inches". Not that it's really required, but...
Thanks Bastian!
Bastien I mean...