Skip to content

Commit

Permalink
Make altitude component of GpsInfo optional
Browse files Browse the repository at this point in the history
This matches the real-world behaviour of apps that only set lat/long when
drawing points on a map.

What made this tricky is that up until gexiv2 0.12.2, this was not necessary.
If altitude (or indeed latitude or longitude) were missing, the function would
still return successfully overall as long as at least one piece of data could
be found. However in 0.12.2 this changes so that *all three* components need
to be present for `gexiv2_metadat_get_gps_info` to return `TRUE`.

I am not sure if this change was intentional, so I have filed a bug upstream
with gexiv2, but in the meantime we still need a workaround. Bug report:
  https://gitlab.gnome.org/GNOME/gexiv2/-/issues/72

Debian Stable as of today ships with 0.12.1, so I couldn't reproduce or test
this locally, but the CircleCI Mac OSX executor is installing a recent version
of the library via homebrew, so if the tests pass there it should be good.

This patch works around the problem by making the altitude (and only altitude)
component of `GpsInfo` `Optional`. This is slightly annoyingly asymmetrical,
but makes sense in reality as there's no reason to have, say, a latitude and
an altitude but not a longitude. The workaround relies on the implementation
detail that gexiv2 upstream happens to check and set latitude and longitude
first, and altitude last. That means that even if we get a `FALSE` return,
the latitude and longitude pointers may nonetheless have been populated, if
the problem was simply that altitude was unset.

For compatibility with various versions of gexiv2, we handle both cases: the
newer behaviour that returns `FALSE`, and also the older behaviour that
returns `TRUE` but has altitude set to `0.0`.

Addresses bug report #42 - #42

References:
  - 0.12.1 version of the code: https://gitlab.gnome.org/GNOME/gexiv2/blob/gexiv2-0.12.1/gexiv2/gexiv2-metadata-gps.cpp#L186
  - 0.12.2 version: https://gitlab.gnome.org/GNOME/gexiv2/blob/gexiv2-0.12.2/gexiv2/gexiv2-metadata-gps.cpp#L246
  - Commit that made the change: https://gitlab.gnome.org/GNOME/gexiv2/-/commit/f2d96b4df221d922ededfbff9236636273a56029
  • Loading branch information
felixc committed Jan 23, 2023
1 parent 18008e5 commit e8ceff8
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ SPDX-License-Identifier: CC0-1.0

## [NEXT] - Unreleased
* Bugfix: Stop reducing/simplifying rational values like apertures.
* Breaking API change/bugfix: The altitude part of GpsInfo is now Optional.
This works around an upstream change that caused `get_gps_info` to return
`None` if altitude was unset, even if lat/long were present. Now lat/long
will be returned, but altitude will be `None`. Altitudes that may have been
previously shown as `0.0` will now be `None`. Calls to `set_gps_info` will
also need updating. Thank you Jonas Hagen for the report and investigation.

## [v0.10.0] - 2023-01-21
* New API: `new_from_app1_segment` allows reading metadata from a buffer.
Expand Down
52 changes: 48 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ pub struct PreviewImage<'a> {
pub struct GpsInfo {
pub longitude: f64,
pub latitude: f64,
pub altitude: f64,
pub altitude: Option<f64>,
}

/// The possible data types that a tag can have.
Expand Down Expand Up @@ -1129,24 +1129,68 @@ impl Metadata {
// GPS-related methods.

/// Retrieve the stored GPS information from the loaded file.
///
/// # Examples
/// ```
/// # let minipng = [137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0,
/// # 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 58, 126, 155, 85, 0, 0, 0, 10, 73, 68, 65,
/// # 84, 8, 215, 99, 248, 15, 0, 1, 1, 1, 0, 27, 182, 238, 86, 0, 0, 0, 0, 73,
/// # 69, 78, 68, 174, 66, 96, 130];
/// # let meta = rexiv2::Metadata::new_from_buffer(&minipng).unwrap();
/// meta.set_gps_info(&rexiv2::GpsInfo { longitude: 0.2, latitude: 0.3, altitude: Some(0.4) });
/// assert_eq!(
/// meta.get_gps_info(),
/// Some(rexiv2::GpsInfo { longitude: 0.2, latitude: 0.3, altitude: Some(0.4) }),
/// );
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn get_gps_info(&self) -> Option<GpsInfo> {
let lon = &mut 0.0;
let lat = &mut 0.0;
let alt = &mut 0.0;
match unsafe { gexiv2::gexiv2_metadata_get_gps_info(self.raw, lon, lat, alt) } {
0 => None,
_ => Some(GpsInfo { longitude: *lon, latitude: *lat, altitude: *alt }),
0 => {
// If we get a "false" result, it might just be because the altitude is missing.
// This is the behaviour since gexiv2 0.12.2. In 0.12.1 and earlier, we'd still
// get a "true" result but altitude would be set to 0.0, which we handle below.
if *lon != 0.0 && *lat != 0.0 && *alt == 0.0 {
Some(GpsInfo { longitude: *lon, latitude: *lat, altitude: None })
} else {
None
}
}
_ => Some(GpsInfo {
longitude: *lon,
latitude: *lat,
altitude: if *alt != 0.0 { Some(*alt) } else { None },
}),
}
}

/// Save the specified GPS values to the metadata.
///
/// # Examples
/// ```
/// # let minipng = [137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0,
/// # 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 58, 126, 155, 85, 0, 0, 0, 10, 73, 68, 65,
/// # 84, 8, 215, 99, 248, 15, 0, 1, 1, 1, 0, 27, 182, 238, 86, 0, 0, 0, 0, 73,
/// # 69, 78, 68, 174, 66, 96, 130];
/// # let meta = rexiv2::Metadata::new_from_buffer(&minipng).unwrap();
/// assert_eq!(meta.get_gps_info(), None);
/// meta.set_gps_info(&rexiv2::GpsInfo { longitude: 0.8, latitude: 0.9, altitude: None });
/// assert_eq!(
/// meta.get_gps_info(),
/// Some(rexiv2::GpsInfo { longitude: 0.8, latitude: 0.9, altitude: None }),
/// );
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn set_gps_info(&self, gps: &GpsInfo) -> Result<()> {
unsafe {
int_bool_to_result(gexiv2::gexiv2_metadata_set_gps_info(
self.raw,
gps.longitude,
gps.latitude,
gps.altitude,
gps.altitude.unwrap_or(0.0),
))
}
}
Expand Down

0 comments on commit e8ceff8

Please sign in to comment.