Repository navigation
Parse a GPX file from the file, not from a copy of its content - #927
Merged
Merged
Conversation
A logger leaves NUL bytes in its log after a power cut, such as on an SD card. mapillary#915 took one NUL byte in the first 64 KiB of a file for the sign of a binary file, and failed the whole log as "not a valid NMEA file: it is not text", where earlier versions read it. With 4 KiB of NUL bytes appended to the NMEA log of an image sequence, all 113 images failed, where the log without them geotags 106, and the other 7 are duplicates. A longer log with the same tail was still read, only because its first 64 KiB hold no NUL byte. Take a file for one that is no NMEA log at all only if its first 64 KiB hold no RMC or GGA sentence, and NUL bytes or mostly bytes that do not decode as UTF-8. A video given as the NMEA file still fails at once: on 1.2 and 4.0 GB videos in 0.7 to 1.0 s and 61 MB, as before; no video or image among 18,023 sample files holds an RMC or GGA sentence in its first 64 KiB. NUL bytes now end a line, as no sentence holds one: a sentence that a logger wrote on after them is read, and one with NUL bytes inside it is skipped under the warning of malformed sentences. A UTF-16 log, which some Windows tools write, is still not read, but the error now says it is UTF-16 text, and to save it as UTF-8 or ASCII. The NUL-padded log of the image sequence now gives the same 106 geotagged images and 7 duplicates as the log without the NUL bytes, with identical descriptions. So does a video log with the same tail. The NMEA logs without NUL bytes, process of images, of GoPro MAX and BlackVue videos, video_process by distance and with an NMEA log, give the same results as before.
mapillary#913 took a sentence without its checksum for cut short when it had fewer fields than a complete sentence: 11 for RMC and 14 for GGA. The fields after the date of an RMC sentence (the magnetic variation, the mode and the navigational status) and after the altitude of a GGA sentence (the height of the geoid and the DGPS fields) are optional, and receivers and converters leave them off. A log without checksums whose sentences end there failed whole, where earlier versions read it: a 650-fix log written with 9 RMC and 12 GGA fields, 10 and 12, 9 and 14, or 11 and 10, gave "1300 of its 1300 RMC and GGA sentences are malformed" or "650 of its 1300", and no track. Require only the fields that a fix is read from: those of an RMC sentence through the date, 9, and those of a GGA sentence through the altitude and its unit, 10. A sentence cut inside the date or the altitude still has fewer, or a date that is not 6 digits, and is still skipped under the warning of malformed sentences, with the reason "cut short: 9 of the 10 fields of a GGA sentence that a fix is read from". Each of those four logs now gives the track of the original, 650 points over 649 s, with no warning. Written GGA first across midnight and ending with an RMC sentence cut inside its date, the log still gives the 650 points, 0 to 649000 ms, with one warning. With a checksum on one line only, with checksums on its GGA sentences only, and as two halves with and without checksums, it reads in full as before. A log whose sentences are all cut short still fails with the reason. The NMEA logs of a Sony action camera and of an image sequence, process of the images and the video, and video_process by distance and by interval with the NMEA log write the same descriptions as before.
mapillary#920 skips the files that accompany images and videos when they are named along with them, as by selecting all the files of a camera folder, rather than fail them with "Not a supported image or video". Its list left out some that cameras, phones and operating systems write into the same folders: DJI low-resolution proxies (.LRF), Apple photo edits (.AAE), XMP metadata, GPS tracks (.kml, .kmz, .fit), and the Thumbs.db and desktop.ini files of Windows. Named, each of them failed the run (exit 6), so that nothing was uploaded without --skip_process_errors. Add the extensions .xmp, .kml, .kmz, .fit, .lrf and .aae, and the file names Thumbs.db and desktop.ini, all compared case-insensitively, to the files skipped with "Skipping ..., which is not a supported image or video". They stay in the process report as unsupported, as the others do. Any other named file that is neither an image nor a video still fails. On a folder of one image and these eight files, named one by one, process now exits 0 with the image described and eight warnings, where it exited 6 with eight errors. A scan of the same folder, its process report, the report of the named files, and process, video_process and sample_video of a mixed folder, a BlackVue video and a GoPro MAX video do not change.
…given mapillary#917 stopped video_process from warning that --geotag_source_path is ignored when the user had not given it, but one place still did: video_process VIDEO OUT --geotag_source '{"source": "gpx", "source_path": "VIDEO.gpx"}', without --geotag_source_path, printed "The option --geotag_source_path is ignored for source ..." once, from the sampling stage. To work out how the geotagging stage will geotag the samples, it gave the sources the video import path as if it were --geotag_source_path, and so warned of the source that has a path of its own. Give the sources there the video import path as the geotagging stage does, through give_video_import_path(), which leaves a source with a path of its own alone without a word. A source without one gets the video import path as before. On a BlackVue video with a GPX of its own track, by distance and by interval, video_process with that GPX source no longer warns, where it warned once. Given --geotag_source_path, it still warns as before: three times with the source in --geotag_source, twice with it in --video_geotag_source, and process once. The descriptions of these and of plain video_process do not change.
Since mapillary#912, a GPX file is read whole into bytes, to key its parsed tracks by its digest, and decoded into a string that stays alive while gpxpy parses it, beside the copies gpxpy makes itself. Parsing a 163 MB GPX of 498 tracks, the memory allocated peaked at 1982 MB, 155 MB above the 1826 MB before mapillary#897, which handed gpxpy the open file. Hand gpxpy the file opened in text mode again, which it reads whole itself, so that no other copy of its content is kept while it parses. The file is decoded as before: with the locale encoding and universal newlines, as a file opened with open() is. Its digest is computed from chunks of 1 MiB, both to look up its tracks and from the bytes gpxpy reads while it parses, and the tracks are kept by the digest of what was parsed, so that a file changed between the lookup and the parse is not kept under the digest of its earlier content. Parsing that GPX now peaks at 1827 MB of allocations, as before mapillary#897. The process peaks themselves stay at 2.55 GB, as on main, as the parsed tree takes most of them. Sampling 11 BlackVue videos every 30 s against that GPX takes 82 s (90 s on main), and process on them with it as --video_geotag_source takes 22 s with 4 or 16 processes (24 to 25 s on main), with the same peaks as on main. Compared with main on process and video_process with GPX sources and with the default sources, on BlackVue, GoPro MAX, Sony and 70mai videos, the descriptions are the same.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #926; review the last commit.
Since #912, a GPX file is read whole into bytes, to key its parsed
tracks by its digest, and decoded into a string that stays alive while
gpxpy parses it, beside the copies gpxpy makes itself. Parsing a 163 MB
GPX of 498 tracks, the memory allocated peaked at 1982 MB, 155 MB above
the 1826 MB before #897, which handed gpxpy the open file.
Hand gpxpy the file opened in text mode again, which it reads whole
itself, so that no other copy of its content is kept while it parses.
The file is decoded as before: with the locale encoding and universal
newlines, as a file opened with open() is. Its digest is computed from
chunks of 1 MiB, both to look up its tracks and from the bytes gpxpy
reads while it parses, and the tracks are kept by the digest of what
was parsed, so that a file changed between the lookup and the parse is
not kept under the digest of its earlier content.
Parsing that GPX now peaks at 1827 MB of allocations, as before #897.
The process peaks themselves stay at 2.55 GB, as on main, as the parsed
tree takes most of them. Sampling 11 BlackVue videos every 30 s against
that GPX takes 82 s (90 s on main), and process on them with it as
--video_geotag_source takes 22 s with 4 or 16 processes (24 to 25 s on
main), with the same peaks as on main. Compared with main on process
and video_process with GPX sources and with the default sources, on
BlackVue, GoPro MAX, Sony and 70mai videos, the descriptions are the
same.
Unit and integration tests, doctests,
mypy --python-version 3.9,ruff check,ruff format --checkandusort diffare clean.