From 8fc58ccf8418cf1311b7ca807406e1adbd2f0b72 Mon Sep 17 00:00:00 2001 From: Caglar Pir Date: Sat, 24 Oct 2026 11:02:35 +0200 Subject: [PATCH] Match samples to videos of the same name by their paths sample_video writes the samples of a video in a folder named after it, at the video's path relative to the folder of videos it samples: the samples of VIDEOS/x/clip.mp4 go to OUT/x/clip.mp4/. The legacy gopro_videos, blackvue_videos and camm sources, and exiftool_xml with a pattern in video_process, matched samples to videos by the name of that folder alone, so all the samples of videos of the same name in different folders were taken for those of one of them. With two BlackVue videos named clip.mp4 in the folders x and y, `video_process VIDEOS OUT --geotag_source blackvue_videos` placed the 13 samples of one along the track of the other, up to 2.1 km off, and failed the 50 samples of the other, with exit 0; exiftool_xml did the same. Match a sample to the video whose path, relative to the deepest folder that holds all the videos, its folder ends with, the longest of them if several do. A sample whose folder ends with none of them, which could be of any of them, is failed with "Cannot tell which of the 2 videos named clip.mp4 this is a frame of", rather than placed along one of them: such as the samples of one of the videos sampled alone, and geotagged by the folder of both. That is a geotagging error, which a later source can still replace. With a single video of a name, its samples are found wherever they are, as before. A sample that two videos match equally well, such as videos of the same name on different drives, which no path tells apart, is failed the same way. Paths are made absolute before they are compared, also where they do not exist: Path.resolve() leaves those relative on Windows with Python 3.9 and 3.10. On the two videos, the 63 samples are now geotagged by the legacy source and by exiftool_xml as by the video's own GPS (0 m off), and as exiftool_xml geotags the samples of each video alone. So are those of two videos of the same name one above the other (VIDEOS/clip.mp4 and VIDEOS/x/clip.mp4, and VIDEOS/a/clip.mp4 and VIDEOS/a/b/clip.mp4). process of the frames of videos of different names, by their legacy sources, of a single video by its file or folder, of a mixed folder and an image sequence, and video_process with the default sources, the legacy source and exif, do not change. --- .../geotag/geotag_images_from_exiftool.py | 12 +- .../geotag/geotag_images_from_video.py | 55 ++++++- mapillary_tools/utils.py | 116 ++++++++++++-- tests/unit/test_geotag_images_from_video.py | 148 +++++++++++++++++- tests/unit/test_utils.py | 99 +++++++++++- 5 files changed, 406 insertions(+), 24 deletions(-) diff --git a/mapillary_tools/geotag/geotag_images_from_exiftool.py b/mapillary_tools/geotag/geotag_images_from_exiftool.py index c20b84200..c86a11801 100644 --- a/mapillary_tools/geotag/geotag_images_from_exiftool.py +++ b/mapillary_tools/geotag/geotag_images_from_exiftool.py @@ -20,7 +20,7 @@ from ..exiftool_runner import ExiftoolRunner from . import options from .base import GeotagImagesFromGeneric -from .geotag_images_from_video import GeotagImagesFromVideo +from .geotag_images_from_video import describe_ambiguous_sample, GeotagImagesFromVideo from .geotag_videos_from_exiftool import GeotagVideosFromExifToolXML from .image_extractors.exiftool import ImageExifToolExtractor @@ -172,7 +172,9 @@ def _geotag_samples_of( source_path: options.SourcePathOption, ) -> list[types.ImageMetadataOrError]: # Find all video paths that have sample images - samples_by_video = utils.find_all_image_samples(image_paths, video_paths) + samples_by_video, ambiguous_samples = utils.find_image_samples( + image_paths, video_paths + ) video_metadata_or_errors = GeotagVideosFromExifToolXML( source_path, num_processes=self.num_processes @@ -182,9 +184,13 @@ def _geotag_samples_of( video_metadata_or_errors, offset_time=self.offset_time, num_processes=self.num_processes, + samples_by_video=samples_by_video, ).to_description(sample_paths) - return sample_metadata_or_errors + return sample_metadata_or_errors + [ + describe_ambiguous_sample(image_path, candidates) + for image_path, candidates in ambiguous_samples.items() + ] @override def to_description( diff --git a/mapillary_tools/geotag/geotag_images_from_video.py b/mapillary_tools/geotag/geotag_images_from_video.py index e9e5bb6b0..0de475eab 100644 --- a/mapillary_tools/geotag/geotag_images_from_video.py +++ b/mapillary_tools/geotag/geotag_images_from_video.py @@ -32,10 +32,15 @@ def __init__( video_metadatas: T.Sequence[types.VideoMetadataOrError], offset_time: float = 0.0, num_processes: int | None = None, + # The samples of each video, as found among all the videos the samples + # could be of (see utils.find_image_samples), or None to find them + # among the videos of video_metadatas + samples_by_video: T.Mapping[Path, T.Sequence[Path]] | None = None, ): super().__init__(num_processes=num_processes) self.video_metadatas = video_metadatas self.offset_time = offset_time + self.samples_by_video = samples_by_video @override def to_description( @@ -48,9 +53,25 @@ def to_description( self.video_metadatas ) + # Found once among all the videos: a sample of one of several videos + # of the same name is a sample of that one only + if self.samples_by_video is None: + samples_by_video: T.Mapping[Path, T.Sequence[Path]] = ( + utils.find_all_image_samples( + image_paths, + [metadata.filename for metadata in self.video_metadatas], + ) + ) + else: + given_paths = set(image_paths) + samples_by_video = { + video_path: [path for path in paths if path in given_paths] + for video_path, paths in self.samples_by_video.items() + } + for video_error_metadata in video_error_metadatas: video_path = video_error_metadata.filename - sample_paths = list(utils.filter_video_samples(image_paths, video_path)) + sample_paths = list(samples_by_video.get(video_path, [])) LOG.debug( "Found %d sample images from video %s with error: %s", len(sample_paths), @@ -68,7 +89,7 @@ def to_description( for video_metadata in video_metadatas: video_path = video_metadata.filename - sample_paths = list(utils.filter_video_samples(image_paths, video_path)) + sample_paths = list(samples_by_video.get(video_path, [])) LOG.debug( "Found %d sample images from video %s", len(sample_paths), @@ -138,7 +159,7 @@ def to_description( self, image_paths: T.Sequence[Path] ) -> list[types.ImageMetadataOrError]: video_paths = utils.find_videos([self.source_path]) - image_samples_by_video_path = utils.find_all_image_samples( + image_samples_by_video_path, ambiguous_samples = utils.find_image_samples( image_paths, video_paths ) video_paths_with_image_samples = list(image_samples_by_video_path.keys()) @@ -150,6 +171,7 @@ def to_description( video_metadatas, offset_time=self.offset_time, num_processes=self.num_processes, + samples_by_video=image_samples_by_video_path, ) sample_metadatas = geotag.to_description(image_paths) @@ -158,7 +180,9 @@ def to_description( # that can geotag it (e.g. exif after this one) may still do sample_paths = {metadata.filename for metadata in sample_metadatas} return sample_metadatas + [ - types.describe_error_metadata( + describe_ambiguous_sample(image_path, ambiguous_samples[image_path]) + if image_path in ambiguous_samples + else types.describe_error_metadata( exceptions.MapillaryGeoTaggingError( f"Not a frame of any video in {self.source_path}" ), @@ -168,3 +192,26 @@ def to_description( for image_path in image_paths if image_path not in sample_paths ] + + +def describe_ambiguous_sample( + image_path: Path, video_paths: T.Sequence[Path] +) -> types.ErrorMetadata: + """ + Describe a sample that could be of any of the videos of the same name + **video_paths**, as its folder does not tell which (see + utils.find_image_samples), rather than place it along one of them + + >>> describe_ambiguous_sample( + ... Path("clip.mp4/clip_0_000001.jpg"), [Path("x/clip.mp4"), Path("y/clip.mp4")] + ... ).error + MapillaryGeoTaggingError('Cannot tell which of the 2 videos named clip.mp4 this is a frame of') + """ + return types.describe_error_metadata( + exceptions.MapillaryGeoTaggingError( + f"Cannot tell which of the {len(video_paths)} videos named " + f"{video_paths[0].name} this is a frame of" + ), + image_path, + filetype=types.FileType.IMAGE, + ) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index be8fc864e..ec22f9d7e 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -111,23 +111,109 @@ def filter_video_samples( def find_all_image_samples( image_paths: T.Iterable[Path], video_paths: T.Iterable[Path] ) -> dict[Path, list[Path]]: - # TODO: not work with the same filenames, e.g. foo/hello.mp4 and bar/hello.mp4 - video_basenames = {path.name: path for path in video_paths} + """ + The samples of each video among the images, but those that could be of + several videos of the same name (see find_image_samples) + """ + samples_by_video, _ = find_image_samples(image_paths, video_paths) + return samples_by_video + + +def find_image_samples( + image_paths: T.Iterable[Path], video_paths: T.Iterable[Path] +) -> tuple[dict[Path, list[Path]], dict[Path, list[Path]]]: + """ + Find the samples of each video among the images. sample_video() names the + samples of a video after it, in a folder named after it, at the path of the + video relative to the folder of videos it samples (e.g. the samples of + VIDEOS/x/clip.mp4 are OUT/x/clip.mp4/clip_0_000001.jpg, ...). + + Videos of the same name in different folders are told apart by their paths + relative to the deepest folder that holds them all: the folder of a sample + ends with the path of its video, the longest of them if several match. A + sample whose folder ends with none of them, which could be of any of them, + is a sample of none. + + Returns the samples of each video, and the videos that each sample that + could be of several videos could be of. + + >>> images = [ + ... Path("OUT/x/clip.mp4/clip_0_000001.jpg"), + ... Path("OUT/y/clip.mp4/clip_0_000001.jpg"), + ... Path("OUT/clip.mp4/clip_0_000001.jpg"), + ... Path("OUT/x/clip.mp4/other.jpg"), + ... ] + >>> videos = [Path("VIDEOS/x/clip.mp4"), Path("VIDEOS/y/clip.mp4")] + >>> samples, ambiguous = find_image_samples(images, videos) + >>> {video.as_posix(): [p.as_posix() for p in paths] for video, paths in samples.items()} + {'VIDEOS/x/clip.mp4': ['OUT/x/clip.mp4/clip_0_000001.jpg'], 'VIDEOS/y/clip.mp4': ['OUT/y/clip.mp4/clip_0_000001.jpg']} + >>> {image.as_posix(): [p.as_posix() for p in paths] for image, paths in ambiguous.items()} + {'OUT/clip.mp4/clip_0_000001.jpg': ['VIDEOS/x/clip.mp4', 'VIDEOS/y/clip.mp4']} + """ + videos = list(deduplicate_paths(video_paths)) + resolved_videos = [_absolute(video) for video in videos] + try: + root: Path | None = ( + Path(os.path.commonpath([video.parent for video in resolved_videos])) + if resolved_videos + else None + ) + except ValueError: + # On different drives + root = None + + # The videos of each name, with their paths relative to the root + videos_by_name: dict[str, list[tuple[Path, tuple[str, ...]]]] = {} + for video, resolved_video in zip(videos, resolved_videos): + if root is None: + relative_parts: tuple[str, ...] = (resolved_video.name,) + else: + relative_parts = resolved_video.relative_to(root).parts + videos_by_name.setdefault(resolved_video.name, []).append( + (video, relative_parts) + ) - image_samples_by_video_path: dict[Path, list[Path]] = {} + samples_by_video: dict[Path, list[Path]] = {} + ambiguous: dict[Path, list[Path]] = {} for image_path in image_paths: - # If you want to walk an arbitrary filesystem path upwards, - # it is recommended to first call Path.resolve() so as to resolve symlinks and eliminate “..” components. - image_dirname = image_path.resolve().parent.name - if image_dirname in video_basenames: - root, _ = os.path.splitext(image_dirname) - if image_path.name.startswith(root + "_"): - video_path = video_basenames[image_dirname] - image_samples_by_video_path.setdefault(video_path, []).append( - image_path - ) - - return image_samples_by_video_path + # Resolved first, to walk the path upwards: the folder of a sample is + # named after its video + sample_dir = _absolute(image_path).parent + candidates = videos_by_name.get(sample_dir.name, []) + stem, _ = os.path.splitext(sample_dir.name) + if not candidates or not image_path.name.startswith(stem + "_"): + continue + + matching = [ + (video, parts) + for video, parts in candidates + if sample_dir.parts[-len(parts) :] == parts + ] + if matching: + longest = max(len(parts) for _, parts in matching) + best = [video for video, parts in matching if len(parts) == longest] + if 1 < len(best): + # Told apart by no path, as videos on different drives + ambiguous[image_path] = best + continue + video = best[0] + elif len(candidates) == 1: + video, _ = candidates[0] + else: + ambiguous[image_path] = [video for video, _ in candidates] + continue + samples_by_video.setdefault(video, []).append(image_path) + + return samples_by_video, ambiguous + + +def _absolute(path: Path) -> Path: + """ + The absolute path of **path**, with its symbolic links resolved where it + exists. Path.resolve() alone leaves a path that does not exist relative on + Windows with Python 3.9 and 3.10 + """ + return Path(os.path.abspath(path.resolve())) def deduplicate_paths(paths: T.Iterable[Path]) -> T.Generator[Path, None, None]: diff --git a/tests/unit/test_geotag_images_from_video.py b/tests/unit/test_geotag_images_from_video.py index 3392a98df..5b0c95c02 100644 --- a/tests/unit/test_geotag_images_from_video.py +++ b/tests/unit/test_geotag_images_from_video.py @@ -12,8 +12,9 @@ from pathlib import Path import pytest -from mapillary_tools import geo, telemetry, types +from mapillary_tools import exceptions, geo, telemetry, types from mapillary_tools.exif_write import ExifEdit +from mapillary_tools.geotag import geotag_images_from_video from mapillary_tools.geotag.geotag_images_from_video import GeotagImagesFromVideo _PWD = Path(os.path.dirname(os.path.abspath(__file__))) @@ -193,3 +194,148 @@ def test_samples_are_placed_along_a_clock_that_gains(tmp_path: Path): sample.name: pytest.approx(40.0 + 2 * k * 0.0001) for k, sample in enumerate(samples) } + + +def _videos_of_the_same_name( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> dict[Path, types.VideoMetadata]: + """ + Two videos named hello.mp4, in the folders x and y, one at 40° north and + the other at 50°, read as such by the legacy sources + """ + videos = {} + for folder, lat in [("x", 40.0), ("y", 50.0)]: + video_path = tmp_path / "videos" / folder / "hello.mp4" + video_path.parent.mkdir(parents=True) + video_path.touch() + metadata = _video_metadata(video_path, [float(i) for i in range(10)]) + for p in metadata.points: + p.lat += lat - 40.0 + videos[video_path] = metadata + + class StubGeotagVideos: + def __init__(self, *args, **kwargs): + pass + + def to_description(self, video_paths): + return [videos[video_path] for video_path in video_paths] + + monkeypatch.setattr( + geotag_images_from_video, "GeotagVideosFromVideo", StubGeotagVideos + ) + return videos + + +def test_samples_of_videos_of_the_same_name_are_placed_by_their_own_video( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """ + The samples of videos of the same name in different folders were all + placed along one of them + """ + _videos_of_the_same_name(tmp_path, monkeypatch) + samples = [] + for folder in ["x", "y"]: + # Where sample_video samples videos/x/hello.mp4 from videos/ + sample_dir = tmp_path / "samples" / folder / "hello.mp4" + sample_dir.mkdir(parents=True) + samples.extend( + _sample(sample_dir, k + 1, START_TIME + datetime.timedelta(seconds=2 * k)) + for k in range(2) + ) + + metadatas = geotag_images_from_video.GeotagImageSamplesFromVideo( + tmp_path / "videos", num_processes=0 + ).to_description(samples) + + lat_by_sample = {} + for metadata in metadatas: + assert isinstance(metadata, types.ImageMetadata), metadata + sample = metadata.filename.relative_to(tmp_path / "samples").as_posix() + lat_by_sample[sample] = metadata.lat + assert lat_by_sample == { + "x/hello.mp4/hello_v_000001.jpg": pytest.approx(40.0), + "x/hello.mp4/hello_v_000002.jpg": pytest.approx(40.0002), + "y/hello.mp4/hello_v_000001.jpg": pytest.approx(50.0), + "y/hello.mp4/hello_v_000002.jpg": pytest.approx(50.0002), + } + + +def test_samples_that_could_be_of_several_videos_are_not_placed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """ + Samples whose folder does not tell which of the videos of the same name + they are of were placed along one of them + """ + _videos_of_the_same_name(tmp_path, monkeypatch) + # Where sample_video samples videos/x/hello.mp4 given alone, or + # videos/y/hello.mp4 + sample_dir = tmp_path / "samples" / "hello.mp4" + sample_dir.mkdir(parents=True) + sample = _sample(sample_dir, 1, START_TIME) + + metadatas = geotag_images_from_video.GeotagImageSamplesFromVideo( + tmp_path / "videos", num_processes=0 + ).to_description([sample]) + + assert len(metadatas) == 1 + assert isinstance(metadatas[0], types.ErrorMetadata) + assert isinstance(metadatas[0].error, exceptions.MapillaryGeoTaggingError) + assert str(metadatas[0].error) == ( + "Cannot tell which of the 2 videos named hello.mp4 this is a frame of" + ) + + +def test_samples_of_videos_of_the_same_name_by_their_exiftool_xml( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """ + The exiftool_xml source finds the samples of the videos whose XML its + pattern names the same way + """ + from mapillary_tools.geotag import geotag_images_from_exiftool, options + + videos = _videos_of_the_same_name(tmp_path, monkeypatch) + for video_path in videos: + video_path.with_suffix(".xml").touch() + + class StubGeotagVideos: + def __init__(self, *args, **kwargs): + pass + + def to_description(self, video_paths): + return [videos[video_path] for video_path in video_paths] + + monkeypatch.setattr( + geotag_images_from_exiftool, "GeotagVideosFromExifToolXML", StubGeotagVideos + ) + samples = [] + for folder in ["x", "y"]: + sample_dir = tmp_path / "samples" / folder / "hello.mp4" + sample_dir.mkdir(parents=True) + samples.append(_sample(sample_dir, 1, START_TIME)) + sample_dir = tmp_path / "samples" / "hello.mp4" + sample_dir.mkdir(parents=True) + samples.append(_sample(sample_dir, 1, START_TIME)) + + metadatas = geotag_images_from_exiftool.GeotagImagesFromExifToolWithSamples( + options.SourcePathOption(pattern="%g.xml", source_path=tmp_path / "videos"), + num_processes=0, + )._geotag_samples_by_pattern(samples) + + result_by_sample = { + metadata.filename.relative_to(tmp_path / "samples").as_posix(): ( + str(metadata.error) + if isinstance(metadata, types.ErrorMetadata) + else pytest.approx(metadata.lat) + ) + for metadata in metadatas + } + assert result_by_sample == { + "x/hello.mp4/hello_v_000001.jpg": 40.0, + "y/hello.mp4/hello_v_000001.jpg": 50.0, + "hello.mp4/hello_v_000001.jpg": ( + "Cannot tell which of the 2 videos named hello.mp4 this is a frame of" + ), + } diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index b8f6b8b28..060fc4841 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -51,8 +51,10 @@ def test_find_all_image_samples(): Path("."), ], ) + # Of the videos of the same name, the samples are of the one whose path, + # relative to the folder that holds them all, their folder ends with assert { - Path("hello.mp4"): [ + Path("foo/hello.mp4"): [ Path("foo/hello.mp4/hello_123.jpg"), Path("foo/hello.mp4/hello_.jpg"), ], @@ -60,6 +62,101 @@ def test_find_all_image_samples(): } == image_samples_by_video_path +def test_find_image_samples_where_resolve_leaves_paths_relative(monkeypatch): + """ + Path.resolve() leaves a path that does not exist relative on Windows with + Python 3.9 and 3.10, which took the samples of world.mp4 for those of + foo/world.mp4 + """ + resolve = Path.resolve + monkeypatch.setattr( + Path, + "resolve", + lambda self, strict=False: resolve(self) if self.exists() else self, + ) + samples, ambiguous = utils.find_image_samples( + [Path("foo/world.mp4/world_123.jpg"), Path("world.mp4/world_123.jpg")], + # "." exists, so it alone was made absolute + [Path("foo/world.mp4"), Path("world.mp4"), Path(".")], + ) + assert { + Path("foo/world.mp4"): [Path("foo/world.mp4/world_123.jpg")], + Path("world.mp4"): [Path("world.mp4/world_123.jpg")], + } == samples + assert {} == ambiguous + + +def test_find_image_samples_of_videos_on_different_drives(monkeypatch): + """Videos of the same name that no path tells apart could be any of them""" + + def commonpath(paths): + raise ValueError("Paths don't have the same drive") + + monkeypatch.setattr(utils.os.path, "commonpath", commonpath) + samples, ambiguous = utils.find_image_samples( + [Path("world.mp4/world_123.jpg")], + [Path("foo/world.mp4"), Path("world.mp4")], + ) + assert {} == samples + assert { + Path("world.mp4/world_123.jpg"): [Path("foo/world.mp4"), Path("world.mp4")] + } == ambiguous + + +def test_find_image_samples_of_videos_of_the_same_name(): + """ + The samples of videos of the same name in different folders were all taken + for the samples of one of them + """ + videos = [ + Path("videos/clip.mp4"), + Path("videos/x/clip.mp4"), + Path("videos/y/clip.mp4"), + Path("videos/y/z/clip.mp4"), + ] + # Where sample_video samples them, from videos/ + images = [ + Path("out/clip.mp4/clip_0_000001.jpg"), + Path("out/x/clip.mp4/clip_0_000001.jpg"), + Path("out/x/clip.mp4/clip_0_000002.jpg"), + Path("out/y/clip.mp4/clip_0_000001.jpg"), + Path("out/y/z/clip.mp4/clip_0_000001.jpg"), + ] + + samples_by_video, ambiguous = utils.find_image_samples(images, videos) + + assert samples_by_video == { + Path("videos/clip.mp4"): [Path("out/clip.mp4/clip_0_000001.jpg")], + Path("videos/x/clip.mp4"): [ + Path("out/x/clip.mp4/clip_0_000001.jpg"), + Path("out/x/clip.mp4/clip_0_000002.jpg"), + ], + Path("videos/y/clip.mp4"): [Path("out/y/clip.mp4/clip_0_000001.jpg")], + Path("videos/y/z/clip.mp4"): [Path("out/y/z/clip.mp4/clip_0_000001.jpg")], + } + assert ambiguous == {} + + +def test_find_image_samples_that_could_be_of_several_videos(): + videos = [Path("videos/x/clip.mp4"), Path("videos/y/clip.mp4")] + # Not where sample_video samples either of them, from videos/ + images = [ + Path("out/clip.mp4/clip_0_000001.jpg"), + Path("out/w/clip.mp4/clip_0_000001.jpg"), + ] + + samples_by_video, ambiguous = utils.find_image_samples(images, videos) + + assert samples_by_video == {} + assert ambiguous == {image: videos for image in images} + + # With one video of the name, its samples are found wherever they are, as + # before + samples_by_video, ambiguous = utils.find_image_samples(images, videos[:1]) + assert samples_by_video == {videos[0]: images} + assert ambiguous == {} + + def test_deduplicates(): pwd = Path(".").resolve() # TODO: life too short to test for windows