From 9c065d20a3af622ab186e000801e8f324b542a4b Mon Sep 17 00:00:00 2001 From: swackhamer Date: Sat, 1 Aug 2026 22:57:33 -0500 Subject: [PATCH] fix(exif): FNumber prints through PrintFNumber, not the raw quotient `ExifIFD:FNumber` disagreed with ExifTool in 1,035 of the 4,238 sample-corpus files: `4` where ExifTool prints `4.0`, and `0.640234375` where it prints `0.64`. Exif.pm 0x829d (line 1853) is `PrintConv => 'Image::ExifTool::Exif::PrintFNumber($val)'`, and `format_tag_value` had no arm for it. The rational therefore fell through to the PrintConv-less-rational rule, which prints the quotient the rational reader already rounded -- `RoundFloat($val, 10)`, i.e. `%.10g`. That is the right rule for a tag with no PrintConv and the wrong one here, in both directions: it drops the decimal place `%.1f` keeps (964 files), and prints the full stored expansion where ExifTool rounds (another 71). PrintFNumber, Exif.pm:5715, reproduced exactly: my $val = shift; if (Image::ExifTool::IsFloat($val) and $val > 0) { # round to 1 decimal place, or 2 for values < 1.0 $val = sprintf(($val<1 ? "%.2f" : "%.1f"), $val); } return $val; Both halves of that condition are load-bearing. Below 1.0 it is two decimal places, not one -- GPS.jpg stores 0.640234375 and ExifTool prints `0.64`. And `$val > 0` guards the sprintf, so a zero is returned unformatted: CanonEOS20Da.jpg, NikonSUPER_COOLSCAN9000ED.jpg and SonyNEX-VG900.jpg store a zero FNumber and ExifTool prints `0`, never `0.0`. This is a *display* conversion and 0x829d has no ValueConv, so the one Composite that consumes FNumber -- `Aperture` (Exif.pm:4782, `ValueConv => '$val[0] || $val[1]'`, which applies PrintFNumber itself) -- must keep receiving the raw quotient. It does: composites are derived from the stored `TagValue` before `format_tag_value` runs at all, and FNumber is deliberately not in `apex_value_conv`. Measured per file, keyed `Group1:Name`, against ExifTool 13.59 over the 4,238-file corpus (oxidex reads 4,104 of them): matched set +1,030 / -0 files 1,030 improved, 0 regressed keys changed ExifIFD:FNumber, and nothing else -- no maker-note `*:FNumber` moved, because the arm is gated on the rational form the EXIF reader produces Composite:* 0 values changed, across all 35 Composite keys and the 3,719 files carrying Composite:Aperture The five files still short of ExifTool are pre-existing gaps this does not touch: CanonRaw.cr3 does not emit the tag, and four Samsung Anycall files store FNumber as a two-element rational array whose second element has a zero denominator (`3 0` vs ExifTool's `3 undef`) -- the array path, not this one. Mutation-tested: removing the arm fails `fnumber_reaches_print_fnumber_from_its_rational` with `4` for `4/1`, and replacing the two-branch sprintf with an unconditional `%.1f` fails `quotients_round_to_one_place_or_two_below_one` (`0.6` for `0.64`) and `non_positive_values_are_returned_unformatted` (`0.0` for `0`). Co-Authored-By: Claude Opus 5 --- src/core/exiftool_compat.rs | 100 +++++++++++++++++++++- src/core/formatters/exif_print_conv.rs | 112 ++++++++++++++++++++++++- 2 files changed, 210 insertions(+), 2 deletions(-) diff --git a/src/core/exiftool_compat.rs b/src/core/exiftool_compat.rs index fd8203140..b3098fe39 100644 --- a/src/core/exiftool_compat.rs +++ b/src/core/exiftool_compat.rs @@ -57,7 +57,9 @@ //! ``` use crate::core::binary_decoders::decode_user_comment; -use crate::core::formatters::exif_print_conv::{print_exposure_time, print_fraction}; +use crate::core::formatters::exif_print_conv::{ + print_exposure_time, print_f_number, print_fraction, +}; use crate::core::formatters::gps_speed_ref::format_gps_dest_distance_ref; use crate::core::formatters::gps_status::{ format_gps_differential, format_gps_measure_mode, format_gps_status, @@ -832,6 +834,38 @@ pub fn format_tag_value(tag_name: &str, value: &TagValue) -> TagValue { )); } + // --------------------------------------------------------------------- + // Rule 19f: FNumber (Exif.pm:1853-1858) + // 0x829d => { Name => 'FNumber', Writable => 'rational64u', + // PrintConv => 'Image::ExifTool::Exif::PrintFNumber($val)', + // PrintConvInv => '$val' } + // + // 0x829d had no arm at all, so its rational fell through to Rule 20's + // `%.10g` quotient. That drops the decimal place ExifTool's `%.1f` keeps + // (`4` where ExifTool prints `4.0` -- 964 sample-corpus files) and prints + // the full stored expansion where ExifTool rounds (`2.638671875` for + // `2.6`, `0.640234375` for `0.64` -- another 71). + // + // This is a *display* conversion, and 0x829d has no ValueConv: the + // Composite `Aperture` (Exif.pm:4782, `ValueConv => '$val[0] || $val[1]'`) + // reads the raw quotient, not this string, and applies `PrintFNumber` + // itself. Composites are derived before `format_tag_value` runs at all -- + // `composite::lookup_key` reads the stored `TagValue` -- so this arm + // cannot reach them, which the corpus run confirms: no `Composite:*` value + // changes. + // --------------------------------------------------------------------- + if base_name == "FNumber" + && let TagValue::Rational { + numerator, + denominator, + } = value + && *denominator != 0 + { + return TagValue::String(print_f_number( + f64::from(*numerator) / f64::from(*denominator), + )); + } + // --------------------------------------------------------------------- // Rule 20: PrintConv-less rationals // @@ -1528,6 +1562,70 @@ mod tests { ); } + /// The dispatch, not just the formatter. + /// + /// `print_f_number` being right proves nothing on its own: 0x829d had no + /// arm in this chain at all, so its rational reached Rule 20 and printed + /// the `%.10g` quotient. 1,035 sample-corpus files reported `4` where + /// ExifTool reports `4.0`, and `0.640234375` where it reports `0.64`. + #[test] + fn fnumber_reaches_print_fnumber_from_its_rational() { + let rational = |n: i32, d: i32| TagValue::Rational { + numerator: n, + denominator: d, + }; + // The whole f-stops, which Rule 20 printed without a decimal place. + for (n, d, want) in [(4, 1, "4.0"), (8, 1, "8.0"), (2, 1, "2.0"), (11, 1, "11.0")] { + assert_eq!( + format_tag_value("ExifIFD:FNumber", &rational(n, d)), + TagValue::String(want.to_string()), + "FNumber {n}/{d} did not reach PrintFNumber" + ); + } + // The rounding cases, which Rule 20 printed in full. + // FujiFilmFinePixA345.jpg stores 344/100. + assert_eq!( + format_tag_value("ExifIFD:FNumber", &rational(344, 100)), + TagValue::String("3.4".to_string()) + ); + // GPS.jpg stores 3277/5119 -- below 1.0, so two decimal places. + assert_eq!( + format_tag_value("ExifIFD:FNumber", &rational(3277, 5119)), + TagValue::String("0.64".to_string()) + ); + // A stored zero stays `0`; ExifTool never prints `0.0` for this tag. + assert_eq!( + format_tag_value("ExifIFD:FNumber", &rational(0, 10)), + TagValue::String("0".to_string()) + ); + // A zero denominator is Rule 17's, and still is. + assert_eq!( + format_tag_value("ExifIFD:FNumber", &rational(4, 0)), + TagValue::String("undef".to_string()) + ); + } + + /// The Composite input is the raw quotient, not this display string. + /// + /// Exif.pm:4782's Composite `Aperture` is `ValueConv => '$val[0] || $val[1]'` + /// over `Desire => { 0 => 'FNumber', 1 => 'ApertureValue' }`, and applies + /// `PrintFNumber` itself. 0x829d has no ValueConv, so what the composite + /// must see is the unrounded rational -- which is why this conversion + /// belongs here, in the display layer, and not in the value the map holds. + /// `apex_value_conv` is the list of tags whose *stored* value is not what + /// a reader wants, and FNumber is deliberately not one of them. + #[test] + fn fnumber_has_no_value_conv_so_composites_keep_the_raw_quotient() { + let stored = TagValue::Rational { + numerator: 3277, + denominator: 5119, + }; + assert_eq!(apex_value_conv("FNumber", &stored), None); + // ...unlike the APEX-stored aperture tags beside it. + assert!(apex_value_conv("ApertureValue", &stored).is_some()); + assert!(apex_value_conv("MaxApertureValue", &stored).is_some()); + } + // ------------------------------------------------------------------------- // strip_family_prefix tests // ------------------------------------------------------------------------- diff --git a/src/core/formatters/exif_print_conv.rs b/src/core/formatters/exif_print_conv.rs index e0577c937..7a326c850 100644 --- a/src/core/formatters/exif_print_conv.rs +++ b/src/core/formatters/exif_print_conv.rs @@ -8,7 +8,7 @@ //! ExifTool for the same seconds, and nine `print_fraction` functions of which //! four printed a different string than ExifTool for the same EV. -use crate::core::formatters::numeric_precision::perl_g; +use crate::core::formatters::numeric_precision::{exiftool_rational_number, perl_g}; /// Port of `Image::ExifTool::Exif::PrintExposureTime` (Exif.pm:5606). /// @@ -189,6 +189,68 @@ pub fn print_fraction(val: f64) -> String { } } +/// Port of `Image::ExifTool::Exif::PrintFNumber` (Exif.pm:5715). +/// +/// ```text +/// sub PrintFNumber($) +/// { +/// my $val = shift; +/// if (Image::ExifTool::IsFloat($val) and $val > 0) { +/// # round to 1 decimal place, or 2 for values < 1.0 +/// $val = sprintf(($val<1 ? "%.2f" : "%.1f"), $val); +/// } +/// return $val; +/// } +/// ``` +/// +/// Referenced by `Exif.pm` 0x829d `FNumber`, by the Composite `Aperture`, and +/// by twenty-odd MakerNote tables. Two details are load-bearing: +/// +/// - **The decimal place is not optional.** `%.1f` prints `4.0` for a stored +/// `4/1`, and 964 sample-corpus files reported a bare `4` because the tag +/// had no PrintConv here at all and fell through to the generic +/// PrintConv-less-rational rule -- `RoundFloat($val, 10)`, i.e. `%.10g`. +/// - **Below 1.0 it is two places, not one.** `GPS.jpg` stores +/// `0.640234375` and ExifTool prints `0.64`; a single `%.1f` would print +/// `0.6`. +/// +/// The `$val > 0` guard is equally load-bearing in the other direction: +/// `CanonEOS20Da.jpg`, `NikonSUPER_COOLSCAN9000ED.jpg` and `SonyNEX-VG900.jpg` +/// store a zero FNumber, and ExifTool prints `0`, not `0.0`. A value that +/// fails the guard is returned as-is, which for a rational is the quotient +/// `GetRational64u` already rounded -- `%.10g`, i.e. +/// [`exiftool_rational_number`]. +/// +/// `IsFloat($val)` is not modelled: this takes an `f64`, so the string +/// predicate is always true for the values that reach it. A zero denominator +/// never gets here -- callers turn that into `undef` first. +/// +/// # Examples +/// +/// ``` +/// use oxidex::core::formatters::exif_print_conv::print_f_number; +/// +/// assert_eq!(print_f_number(4.0), "4.0"); +/// assert_eq!(print_f_number(2.8), "2.8"); +/// // Rounds; it does not print the stored binary expansion +/// assert_eq!(print_f_number(2.638671875), "2.6"); +/// // Below 1.0 gets a second decimal place +/// assert_eq!(print_f_number(0.640234375), "0.64"); +/// // Not > 0: returned unchanged, so `0` and not `0.0` +/// assert_eq!(print_f_number(0.0), "0"); +/// ``` +pub fn print_f_number(val: f64) -> String { + if val > 0.0 { + if val < 1.0 { + format!("{:.2}", val) + } else { + format!("{:.1}", val) + } + } else { + exiftool_rational_number(val) + } +} + #[cfg(test)] mod tests { use super::*; @@ -251,4 +313,52 @@ mod tests { assert_eq!(print_exposure_time_micros_str("500000"), "0.5"); assert_eq!(print_exposure_time_micros_str("2000000"), "2"); } + + /// `sprintf("%.1f")` at or above 1.0 -- the decimal place is the point. + /// + /// Every one of these is a whole f-stop stored as `N/1`, and the generic + /// PrintConv-less-rational rule (`%.10g`) prints them without it. 964 + /// sample-corpus files reported `4`, `8`, `5`, `2` and so on where + /// ExifTool reports `4.0`, `8.0`, `5.0`, `2.0`. + #[test] + fn whole_f_stops_keep_their_decimal_place() { + assert_eq!(print_f_number(1.0), "1.0"); + assert_eq!(print_f_number(2.0), "2.0"); + assert_eq!(print_f_number(4.0), "4.0"); + assert_eq!(print_f_number(8.0), "8.0"); + assert_eq!(print_f_number(11.0), "11.0"); + assert_eq!(print_f_number(45.0), "45.0"); + } + + /// The rounding, at both precisions. + /// + /// These are the stored quotients of real corpus files, and each one is a + /// value `%.10g` prints in full: `Apple_iPhone15Pro.jpg` reported + /// `1.779999971`, `FujiFilmFinePixA345.jpg` reported `3.44`, and + /// `GPS.jpg` reported `0.640234375`. + #[test] + fn quotients_round_to_one_place_or_two_below_one() { + assert_eq!(print_f_number(1.779999971), "1.8"); + assert_eq!(print_f_number(3.44), "3.4"); + assert_eq!(print_f_number(2.638671875), "2.6"); + assert_eq!(print_f_number(2.799804688), "2.8"); + // The `$val < 1` branch: two places, not one. + assert_eq!(print_f_number(0.640234375), "0.64"); + assert_eq!(print_f_number(0.95), "0.95"); + // ...and 1.0 itself is on the `%.1f` side of that boundary. + assert_eq!(print_f_number(0.999), "1.00"); + assert_eq!(print_f_number(1.0), "1.0"); + } + + /// `$val > 0` guards the sprintf, so a zero is returned unchanged. + /// + /// `CanonEOS20Da.jpg`, `NikonSUPER_COOLSCAN9000ED.jpg` and + /// `SonyNEX-VG900.jpg` all store a zero FNumber, and `exiftool -G1 -s` + /// prints `0` for each. Formatting unconditionally would print `0.0`, + /// which ExifTool never emits for this tag. + #[test] + fn non_positive_values_are_returned_unformatted() { + assert_eq!(print_f_number(0.0), "0"); + assert_eq!(print_f_number(-2.8), "-2.8"); + } }