Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for coordinates in Adobe format in XMP tags #653

Merged
merged 12 commits into from
Sep 6, 2023
51 changes: 47 additions & 4 deletions mapillary_tools/exif_read.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import abc
import datetime
import logging
import re
import typing as T
import xml.etree.ElementTree as et
from fractions import Fraction
from pathlib import Path

import exifread
Expand All @@ -22,6 +24,8 @@
EXIFREAD_LOG = logging.getLogger("exifread")
EXIFREAD_LOG.setLevel(logging.ERROR)

adobe_format_regex = re.compile(r"(\d+),(\d{1,3}\.?\d*)([NSWE])")


def eval_frac(value: Ratio) -> float:
return float(value.num) / float(value.den)
Expand All @@ -47,6 +51,39 @@ def gps_to_decimal(values: T.Tuple[Ratio, Ratio, Ratio]) -> T.Optional[float]:
return degrees + minutes / 60 + seconds / 3600


def _parse_coord_numeric(coord: str) -> T.Optional[float]:
try:
return float(coord)
except ValueError:
return None


def _parse_coord_adobe(coord: str) -> T.Optional[float]:
"""
Parse Adobe coordinate format: <degrees,fractionalminutes[NSEW]>
"""
matches = adobe_format_regex.match(coord)
if matches:
sign = {"N": 1, "S": -1, "E": 1, "W": -1}
deg = Ratio(int(matches.group(1)), 1)
min_frac = Fraction.from_float(float(matches.group(2)))
min = Ratio(min_frac.numerator, min_frac.denominator)
sec = Ratio(0, 1)
converted = gps_to_decimal((deg, min, sec))
if converted is not None:
return converted * sign[matches.group(3)]
return None


def _parse_coord(coord: T.Optional[str]) -> T.Optional[float]:
if coord is None:
return None
parsed = _parse_coord_numeric(coord)
if parsed is None:
parsed = _parse_coord_adobe(coord)
return parsed


def _parse_iso(dtstr: str) -> T.Optional[datetime.datetime]:
try:
return datetime.datetime.fromisoformat(dtstr)
Expand Down Expand Up @@ -378,20 +415,26 @@ def extract_direction(self) -> T.Optional[float]:
)

def extract_lon_lat(self) -> T.Optional[T.Tuple[float, float]]:
lat = self._extract_alternative_fields(["exif:GPSLatitude"], float)
lat_str: T.Optional[str] = self._extract_alternative_fields(
["exif:GPSLatitude"], str
)
lat: T.Optional[float] = _parse_coord(lat_str)
if lat is None:
return None

lon = self._extract_alternative_fields(["exif:GPSLongitude"], float)
lon_str: T.Optional[str] = self._extract_alternative_fields(
["exif:GPSLongitude"], str
)
lon = _parse_coord(lon_str)
if lon is None:
return None

ref = self._extract_alternative_fields(["exif:GPSLongitudeRef"], str)
if ref and ref.upper() == "W":
if ref and ref.upper() == "W" and lon > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to apply the reference for the adobe value again? I thought adobe has NSEW parsed from its own format so it doesn't have to respect this one here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the Adobe coords already have the right sign, that's why I added the check on lon/lat > 0 - to avoid multiplying them by -1 again. But yeah, it's hacky, I'll make it more explicit :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not separating them? So adoble values don't need to respect the ref tags. See my proposal above.

lon = -1 * lon

ref = self._extract_alternative_fields(["exif:GPSLatitudeRef"], str)
if ref and ref.upper() == "S":
if ref and ref.upper() == "S" and lat > 0:
lat = -1 * lat

return lon, lat
Expand Down
4 changes: 4 additions & 0 deletions mapillary_tools/exiftool_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,10 @@ def extract_lon_lat(self) -> T.Optional[T.Tuple[float, float]]:
if lon_lat is not None:
return lon_lat

lon_lat = self._extract_lon_lat("XMP-exif:GPSLongitude", "XMP-exif:GPSLatitude")
if lon_lat is not None:
return lon_lat

return None

def _extract_lon_lat(
Expand Down
Binary file added tests/data/adobe_coords/adobe_coords.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions tests/integration/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@
"MAPDeviceModel": "VIRB 360",
"MAPOrientation": 1,
},
"adobe_coords.jpg": {
malconsei marked this conversation as resolved.
Show resolved Hide resolved
"filetype": "image",
"MAPLatitude": -0.0702668,
"MAPLongitude": 34.3819352,
"MAPCaptureTime": "2019_07_16_10_26_11_000",
"MAPCompassHeading": {"TrueHeading": 0, "MagneticHeading": 0},
"MAPDeviceMake": "SAMSUNG",
"MAPDeviceModel": "SM-C200",
"MAPOrientation": 1,
},
}


Expand Down Expand Up @@ -260,6 +270,27 @@ def test_angle_with_offset_with_exiftool(setup_data: py.path.local):
return test_angle_with_offset(setup_data, use_exiftool=True)


def test_parse_adobe_coordinates(setup_data: py.path.local):
args = f"{EXECUTABLE} process --file_types=image {PROCESS_FLAGS} {setup_data}/adobe_coords"
x = subprocess.run(args, shell=True)
verify_descs(
[
{
"filename": str(Path(setup_data, "adobe_coords", "adobe_coords.jpg")),
"filetype": "image",
"MAPLatitude": -0.0702668,
"MAPLongitude": 34.3819352,
"MAPCaptureTime": _local_to_utc("2019-07-16T10:26:11"),
"MAPCompassHeading": {"TrueHeading": 0.0, "MagneticHeading": 0.0},
"MAPDeviceMake": "SAMSUNG",
"MAPDeviceModel": "SM-C200",
"MAPOrientation": 1,
}
],
Path(setup_data, "adobe_coords/mapillary_image_description.json"),
)


def test_zip(tmpdir: py.path.local, setup_data: py.path.local):
zip_dir = tmpdir.mkdir("zip_dir")
x = subprocess.run(
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_process_and_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def test_process_and_upload_images_only(
setup_upload: py.path.local,
):
x = subprocess.run(
f"{EXECUTABLE} --verbose process_and_upload --filetypes=image {UPLOAD_FLAGS} {PROCESS_FLAGS} {setup_data} {setup_data} {setup_data}/images/DSC00001.JPG --desc_path=-",
f"{EXECUTABLE} --verbose process_and_upload --filetypes=image {UPLOAD_FLAGS} {PROCESS_FLAGS} {setup_data}/images {setup_data}/images {setup_data}/images/DSC00001.JPG --desc_path=-",
shell=True,
)
assert x.returncode == 0, x.stderr
Expand Down
25 changes: 24 additions & 1 deletion tests/unit/test_exifread.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
import pytest
from mapillary_tools import geo

from mapillary_tools.exif_read import ExifRead, parse_datetimestr_with_subsec_and_offset
from mapillary_tools.exif_read import (
ExifRead,
_parse_coord,
parse_datetimestr_with_subsec_and_offset,
)
from mapillary_tools.exif_write import ExifEdit
from PIL import ExifTags, Image

import typing as T

"""Initialize all the neccessary data"""

this_file = os.path.abspath(__file__)
Expand Down Expand Up @@ -250,6 +256,23 @@ def test_parse():
assert str(dt) == "2021-10-10 17:29:54.124000-02:00", dt


@pytest.mark.parametrize(
"raw_coord,expected",
[
("0.0", 0),
("foo", None),
("1.5", 1.5),
("-1.5", -1.5),
("33,18.32N", 33.30533),
("33,18.32S", -33.30533),
("44,24.54E", 44.40900),
("44,24.54W", -44.40900),
],
)
def test_parse_coordinates(raw_coord: T.Optional[str], expected: T.Optional[float]):
assert _parse_coord(raw_coord) == pytest.approx(expected)


# test ExifWrite write a timestamp and ExifRead read it back
def test_read_and_write(setup_data: py.path.local):
image_path = Path(setup_data, "test_exif.jpg")
Expand Down