feat(exif): DC-008 tag breadth, per-tag value shape, and value descriptions - #529
Open
justin13888 wants to merge 19 commits into
Open
justin13888 wants to merge 19 commits into
justin13888 wants to merge 19 commits into
Conversation
The `exif_tags!` table carried only (IfdKind, id, name), so nothing said what value shape CIPA DC-008 mandates for a tag: a caller could write a SHORT where the spec demands RATIONAL, or a two-component DateTime, and the crate had no way to notice. Each row now also carries the Type and Count columns of the DC-008 table that defines the tag - Table 6 (0th IFD), Tables 8 and 9 (Exif sub-IFD), Table 14 (GPS), Table 16 (Interoperability) - reachable as `ExifTag::field_types` and `ExifTag::component_count`. `TagCount` spells the Count column's three shapes (a fixed number, `Any`, and SubjectArea's "2 or 3 or 4"), and `TagCount::allows` is the whole of its behaviour. Nine catalogued tags come from other specifications (ApplicationNotes, IPTC-NAA, InterColorProfile, Rating, RatingPercent and the DCF-era Interoperability tags): DC-008 claims nothing for them, so their type list is empty rather than invented, and a test names exactly that set so the boundary cannot drift silently. This is data only - no read or write path consults it yet, so no behaviour changes.
CIPA DC-008 Table 8 section I gained a block after LensSerialNumber in Exif 3.0 - ImageTitle, Photographer, ImageEditor, CameraFirmware, RAWDevelopingSoftware, ImageEditingSoftware and MetadataEditingSoftware (0xA436-0xA43C, sections 4.6.6.9.8 to 4.6.6.9.14) - which the catalogue did not carry, so they read back as unnamed tags even though the crate documents Exif 3.0 as the version it implements. All seven are ASCII-or-UTF-8 with an open count. `ExifTag` is `#[non_exhaustive]`, so the additions are minor; they complete the crate's coverage of the tags DC-008 itself defines.
`Exif::set_tag` writes whatever it is handed, so nothing stopped a
caller emitting a SHORT where CIPA DC-008 demands a RATIONAL, or a
GPSLatitude of two components. Now that each row carries the spec's Type
and Count columns, the write path can say so.
`check_tag` and `set_tag_checked` are the conformance check, and they
live beside the writer on purpose: writing is the only place gamut
authors the bytes, so it is the only place a spec violation is gamut's
to refuse. `Exif::set_tag` and the whole read path are untouched - a
caller reproducing a non-conformant source file must still be able to,
and real files break the spec routinely.
Failures carry `TagConstraintError`, a separate type from `ExifError`:
one reports what is wrong with data being read, the other with a value
being offered for writing. The message quotes CIPA DC-008's own
spellings ("SHORT or LONG", "2 or 3 or 4") so it can be checked against
the specification without a lookup table.
`spec_type_name` maps the on-disk type *code* rather than matching
`FieldType`'s variants: gamut-ifd grows three BigTIFF variants when
another workspace crate turns that feature on and Cargo unifies it into
this build, so a variant match would compile in one configuration and
not the other.
`ExifTag::name` was the only string in the crate: nothing turned `Orientation = 1`, `ResolutionUnit = 2` or the LightSource and MeteringMode enumerations into anything a person can read, which is what separates a metadata library that can display from one that can only round-trip. The new `describe` module carries CIPA DC-008's own wording for every tag the specification enumerates - not exiftool's or exiv2's phrasing, which would put an oracle where the source of truth belongs. `described_values` publishes a tag's whole defined domain and `describe` is exactly a lookup into it, so what counts as a reserved code has one definition rather than two. `Flash` is a bitfield, not an enumeration, so `flash` decomposes Figure 17's five fields instead of tabulating every bit combination. It sits behind an opt-in `describe` feature, default off: the table is several kilobytes of static strings that a consumer which only writes or only round-trips metadata never reads, and the workspace is allocation-conscious by convention. The feature is additive, so no existing build grows; `full` turns it on.
The tag catalogue's names had no external check: a mis-transcribed name would have been asserted only against itself. exiv2, the crate's documented oracle, knows a canonical name for every standard tag, so writing every catalogued tag into a stream and asking exiv2 for `Exif.<group>.<name>` decides whether gamut's name is the one the reference implementation uses. All 160 names resolve except three, which the test pins as an exact set so the divergence can neither grow nor shrink unnoticed, and a second test proves each is a naming difference rather than a missing tag by reading the same value back under exiv2's own spelling: 0x02BC ApplicationNotes vs exiv2's XMLPacket 0x83BB IPTC-NAA vs exiv2's IPTCNAA 0x8827 PhotographicSensitivity vs exiv2's ISOSpeedRatings CIPA DC-008 renamed 0x8827 in Exif 2.3 and exiv2 keeps the Exif 2.2 name; the other two are TIFF/EP-lineage tags no vendored spec defines. DC-008 wins in each case - it is the specification this crate implements, and exiv2 is the oracle, not the source. exiv2 also resolves all seven Exif 3.0 authorship tags this branch added, which is independent confirmation of the ids and names.
STATUS.md and README.md still described the tag dictionary as a partial "standard dictionary" with exiftool-parity breadth deferred. The catalogue is now complete for CIPA DC-008 - 151 tags the spec defines plus nine carried from other specifications - so the deferred item is no longer breadth within DC-008 but breadth beyond any vendored spec, which means transcribing from those specifications rather than copying a table out of the oracle. Adds a P10 phase describing what the catalogue covers, the write-only conformance check, and the `describe` feature; records the three tags where exiv2's name and DC-008's disagree, with both readings, so a reader can see why gamut keeps the spec's; and names the one piece of the issue's acceptance that could not be met - the oracle shim exposes exiv2's raw value but not its interpreted one, so the description tables are checked structurally rather than against exiv2's rendering.
Every other assertion about the description tables reads one through `described_values`, so an arm that lost its rows would make those assertions vacuous rather than failing them. This names the exact set of tags that have a table, which is the one thing that notices. Also guards the oracle helper's string length against underflow: a component count includes the terminating NUL, so a count of zero would have wrapped in a profile without overflow checks instead of failing loudly.
CIPA DC-008 contradicts itself on 0xA407. Table 9's Type column, from which the rest of the catalogue's types were transcribed, says RATIONAL; §4.6.6.7.41 says SHORT and enumerates five integer codes, 0 for None through 4 for High gain down. The tag's own section wins. A fraction cannot carry an enumeration, and both exiv2 and ExifTool read the tag as SHORT, so RATIONAL in the summary table is an erratum. Left as it was, the validating setter would have refused the only values the tag can actually hold. Found by cross-checking all 149 specified rows against the per-tag `Tag = / Type = / Count =` blocks the specification states beside each definition; this was the only contradiction between the two.
…be feature The crate-level documentation is the front door, and it described only the read path. A caller had no way to discover from it that a tag now carries the value shape CIPA DC-008 mandates, that `set_tag_checked` enforces it while `set_tag` deliberately does not, or that the enumerated values have descriptions behind a feature. The `describe` names are written as code rather than intra-doc links: the module does not exist when the feature is off, so a link there would be broken in a default build. Also drops the `exif_tags!` link in the tag module's docs, on a sentence this branch already rewrites. `macro_rules!` items are not in scope at that path, so rustdoc could never resolve it.
…into feat/417-exif-tag-breadth
The tag-name differential added three tests to tests/oracle.rs, taking it from two `#[test]`s to five. `cargo test` runs them on separate threads, and they are the workspace's only callers of the oracle's EXIF entry points, so exiv2 started being entered concurrently for the first time. It segfaulted. The failure is a race, not a bad input: of the four mutation shards CI ran on one commit, each of which runs the identical unmutated baseline, three passed and one died with SIGSEGV before any test reported. Same tree, same exiv2 build, same bytes. `exiv2-oracle` already guards its XMP entry points for exactly this reason — XMPCore keeps global state and is documented as not thread-safe — and leaves its EXIF entry points unguarded, with nothing recording that as a decision. Until that is settled, every exiv2 call this file makes goes through one lock, so exiv2 only ever runs single-threaded here. This is a work-around at the call site: it protects one file, and the next caller of `exif_get` will not know to bring its own lock. The guard belongs in the oracle crate beside the XMP one, filed as #536.
The table said "exiv2 0.28". The submodule pins v0.28.8, and issue #417 quotes 0.28.9's tag count, so a reader comparing the two had no way to tell which build produced these three readings.
STATUS.md said nine tags were "carried from other specifications for compatibility" without saying which, so a reader could not check them against the workspace rule that a row comes from a specification under references/. Three of the nine do: ApplicationNotes and IPTC-NAA from the TIFF table in the vendored XMP Part 3, and InterColorProfile from ICC.1:2001-04, which states the tag number outright. Six do not, and are now named as residuals. DC-008 3.0 lists the four DCF-era Interoperability tags in its original-preservation-image annex but delegates them to DCF Table 13 section 4.7, and DCF is not vendored; Rating and RatingPercent are a Microsoft extension with no vendored text at all. That is precisely why their `field_types` is empty - nothing here fixes a type or count for them, so nothing is claimed. Vendoring DCF would settle four of the six. All nine predate this phase; it only made their status explicit.
…into feat/417-exif-tag-breadth
`describe` answered `None` for GPSLatitudeRef, GPSLongitudeRef, GPSSpeedRef, GPSTrackRef, GPSImgDirectionRef, GPSDestLatitudeRef, GPSDestLongitudeRef, GPSDestBearingRef and GPSDestDistanceRef, though §4.6.7.1 enumerates each in exactly the character-coded shape GPSStatus and GPSMeasureMode already use. Since the module documents `None` as "the spec reserves that code", the crate was stating something false about the specification: `describe(GpsSpeedRef, 'K')` was `None` where DC-008 says "Kilometers per hour". Transcribe all nine from the vendored spec text, bringing the coverage to the 39 tags for which DC-008 prints a value table. The drift guard is corrected in the same commit: it pinned the 30 names the table happened to hold and was worded as if that were what the spec enumerates, so it asserted a false proposition and blocked the fix. A missing match arm is not a mutant, so neither the structural checks nor the mutation gate could have seen this. The one new test covers the class that replaces them: these tags reuse the same few letters for unrelated meanings - 'M' is a magnetic direction, miles per hour, or miles - so an arm that swallowed one tag into a sibling's group would answer plausibly and wrongly while every existing assertion still held.
Four claims were wider than the code: - The README said the catalogue holds "every tag CIPA DC-008 defines". It holds every tag in the five tables that define one; §4.6.3.1.1, §4.6.3.2.1 and §4.6.3.3.1 give the three IFD-pointer tags their own Tag/Type/Count blocks outside those tables. Those stay uncatalogued on purpose - the writer synthesises them from the tree and removes any that were hand-set, so naming them would let `set_tag_checked` accept a pointer that is then discarded. Say so where a reader would ask, at the catalogue and in the README. - `lib.rs` said every catalogued tag carries a spec-mandated type and count. False for the nine rows DC-008 does not define, whose `field_types` is empty by design; match `tag.rs`'s precise wording. - `lib.rs` and `writer.rs` each named one lenient write door where six are public. Enumerate them: `set_tag`, `set`, `exif_ifd_mut`, `gps_ifd_mut`, `interop_ifd_mut` and `set_gps_ifd`, plus the siblings reaching the same directories. - `set_tag_checked` refuses a RATIONAL `GainControl`, which Table 9 literally permits, because §4.6.6.7.41 says SHORT and enumerates five integer codes. Name the contradiction on the setter itself and point at `Exif::set_tag` as the escape hatch, so a caller reproducing a Table-9-conformant file is not left guessing. Also record two properties nobody had written down: `ExifTag`'s discriminants are not stable, because variants are declared in the specification's order and `ALL` iterates it, and the `describe` feature is reachable only from a direct dependent (#543). `TagConstraintError` is deliberately parallel to `ExifError` rather than a variant of it, as `GpsConversionError` already is. The `Type` and `Count` columns are hand-transcribed and nothing re-derives them, so a corrected-but-different value would pass every check (#544).
…into feat/417-exif-tag-breadth
The drift-guard docstring and STATUS.md said the thirty-nine tags with a table here are exactly those for which CIPA DC-008 prints a value table. That is false in both directions: the spec prints one for Flash, GPSVersionID, FlashpixVersion, YCbCrSubSampling and InteroperabilityIndex, none of which has an arm, and prints none for FocalPlaneResolutionUnit, which does. State the rule the code implements instead - a single scalar code, per value or per ComponentsConfiguration element - name the four structurally non-scalar exclusions, and name FocalPlaneResolutionUnit as the deliberate exception with the cross-reference that justifies it. The discriminant-instability note claimed nothing may cast ExifTag to an integer because it is non_exhaustive, carries no repr and is unreachable from gamut-ffi. Only the last is load-bearing: non_exhaustive blocks exhaustive matching, not `as` on a fieldless enum. Keep the warning, drop the false reassurance. The README generalised the pointer-drop past its mechanism: the writer keys on each pointer's home directory, so one hand-set elsewhere survives. Narrow the claim to that. State the lenient-door boundary too - doors that admit a field, of which there are nine - and note set_thumbnail separately as admitting unvalidated bytes with no field.
The drift-guard docstring and STATUS.md each say "Four tags whose sections print one are excluded" and then name five: Flash, GPSVersionID, FlashpixVersion, YCbCrSubSampling and InteroperabilityIndex. The module docs get it right by counting Flash separately - "Four more tags" follows the sentence that names it - but these two fold Flash into the list without adjusting the number, so each contradicts the list beside it. Say five. Also rewrap the one prose line that overran, leaving STATUS.md's only long lines the table rows that cannot be wrapped.
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
1 participant
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 #522 (
feat/419-exif-streaming-reader). Review only the commits above that base; merge #522 first.Summary
gamut-exifcatalogued 153 tags as(IfdKind, id, name)triples.ExifTag::name()was the onlystring in the crate, nothing mapped a tag to the field type and component count CIPA DC-008
requires, and nothing rendered an enumerated value. Issue #417 named all three gaps.
All three are closed, staying inside the vendored specification (
references/exif, CIPA DC-008 /Exif 3.0):
Breadth. A census of DC-008 against the catalogue found the crate was missing exactly seven
tags the spec defines: the Exif 3.0 authorship/software block
ImageTitle,Photographer,ImageEditor,CameraFirmware,RAWDevelopingSoftware,ImageEditingSoftware,MetadataEditingSoftware(0xA436–0xA43C, Table 8 section I, §4.6.6.9.8–§4.6.6.9.14). Withthose added the catalogue covers every one of the 151 tags DC-008 itself defines — Table 6
(30, 0th IFD), Tables 8 and 9 (88, Exif sub-IFD), Table 14 (32, GPS), Table 16 (1,
Interoperability) — plus nine carried from other specifications for compatibility, for 160 in all.
The issue's headline number (exiv2's 408) is not reached, and deliberately so: the remaining
~248 are tags DC-008 does not define (TIFF/EP, DNG, vendor lineages). Transcribing them means
vendoring their specifications, not copying exiv2's table, which would put an oracle where the
source of truth belongs. Filed as #532 with the design question it raises first (those tags may
belong to
gamut-tiff/gamut-dng/gamut-ifd, not here).Value shape. Every row now carries the
TypeandCountcolumns of the DC-008 table thatdefines it, as
ExifTag::field_types()andExifTag::component_count().set_tag_checkedrefusesa value that contradicts them.
Exif::set_tagand the whole read path are unchanged — realfiles break the spec routinely and a caller reproducing one must still be able to. For the nine
non-DC-008 tags
field_types()is empty: no constraint is claimed rather than one invented.Descriptions. A new
describemodule renders the enumerated tags in DC-008's own wording, andflashdecomposes theFlashbitfield of §4.6.6.7.21 Figure 17. Behind an opt-indescribefeature, default off; no existing build turns it on.
One spec defect found and fixed
GainControl(0xA407): DC-008 Table 9'sTypecolumn saysRATIONAL, but §4.6.6.7.41 saysSHORTand enumerates five integer codes (0–4). A fraction cannot carry an enumeration, and bothexiv2 and ExifTool read it as
SHORT, so the summary table is an erratum. Had it been transcribedas written, the validating setter would have refused the only values the tag can hold. Found by
mechanically cross-checking all 149 specified rows against the per-tag
Tag = / Type = / Count =blocks the spec states beside each definition; it was the only contradiction between the two.
How the names are checked
Not against a hand-written list, which would only say the table equals itself.
tests/oracle.rswrites every catalogued tag into a stream and asks exiv2 for
Exif.<group>.<name>. All 160 resolveexcept three, which the test pins as an exact set, with a second test proving each is a naming
difference rather than a missing tag by reading the same value back under exiv2's own spelling:
0x8827PhotographicSensitivity(renamed in Exif 2.3)ISOSpeedRatings(Exif 2.2)0x02BCApplicationNotesXMLPacket0x83BBIPTC-NAAIPTCNAADC-008 wins in each case and both readings are recorded in
STATUS.md. exiv2 also resolves allseven newly added Exif 3.0 tags — independent confirmation of their ids and names.
Where the nine non-DC-008 rows come from
The workspace rule is that a row comes from a specification under
references/. Three of the ninedo; six do not, and are named as residuals — carried because real files and every other reader
use them, with no vendored text fixing a type or count, which is why their
field_types()is emptyand
set_tag_checkedconstrains nothing. All nine predate this branch; only the disclosure is new.ApplicationNotes0x02BCreferences/xmp/xmp-part3.pdf— TIFF table:700 / 0x2BC — XMP packetIPTC-NAA0x83BBreferences/xmp/xmp-part3.pdf— same table:33723 / 0x83BB — IPTC dataset; the payload's own format isreferences/iptc/iim-4.2.pdfInterColorProfile0x8773references/icc/icc.1-2001-04.pdf— "The TIFFTag that identifies the field = 34675(8773.H)"InteroperabilityVersion0x0002,RelatedImageFileFormat0x1000,RelatedImageWidth0x1001,RelatedImageLength0x1002Rating0x4746,RatingPercent0x4749Vendoring DCF would settle four of the six. Recorded in
STATUS.mdand as decision 8 below.What this PR does not prove
Two narrowings, stated plainly because both reduce what the change is evidence for:
against exiv2's interpreted value.
tooling/exiv2-oracleexposesExiv2::Exifdatum::toString()— the raw serialised value,
"1"forOrientation— and has no entry point reachingExifdatum::print(), which is the interpreted one,"top, left". So the names are checkedagainst exiv2 for all 160 tags, and the descriptions are transcribed from CIPA DC-008 and
checked structurally instead: every table ascending with no repeated code,
describeexactly alookup into
described_valuesso "reserved" has one definition, the enumerated-tag set pinned,and each
Flashbit field proved a function of its own bits across all 128 assignedcombinations. Those falsify a transposed or mis-masked table; they cannot falsify a
self-consistently wrong transcription. Only an oracle can. Filed as tooling/exiv2-oracle: expose Exifdatum::print() so a rendered-value differential is possible #533; decision 5 below.
standard-tag count; this PR closes the catalogue against CIPA DC-008 (151 of the 160 rows), and
the ~248 remaining are tags DC-008 does not define. Filed as gamut-exif: tag breadth beyond CIPA DC-008 (the remaining ~248 tags exiv2 knows) #532, which names where they
come from — TIFF/EP, DNG, and vendor lineages — and the design question of whether they belong
in this crate at all rather than in
gamut-tiff/gamut-dng/gamut-ifd. The PR is thereforeRefs #417, notCloses.Validation
Every command was run in this worktree at the head commit.
mise run fmt/fmt-checkneed the__CARGO_TEST_ROOTprefix in a nested checkout (cargo otherwise walks past theworktree root to the primary checkout's manifests); that is an environment artefact, not a change
to any manifest.
git submodule update --init --recursivemise run fetch-av1-oraclesCARGO_BUILD_JOBS=2 cargo test -p gamut-exif --all-featuresCARGO_BUILD_JOBS=2 cargo clippy -p gamut-exif --all-targets --all-features__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmt-checkmise run check-testsconvco check origin/feat/419-exif-streaming-reader..HEADmise run check-release-depsmise run check-ffi-featurescargo doc -p gamut-exif --no-depsand--all-featuresreader.rs:35,stream.rs:7); the third, in atag.rsline this branch rewrites, is fixed heremise run lint(workspace,--all-targets --all-features -D warnings)warning:line is cargo advising that a transitive dependency,proc-macro-error2 v2.0.1, will be rejected by a future rustc; it is pre-existing and unrelated.mise run test(workspace)GAMUT_MUTANTS_BASE=origin/feat/419-exif-streaming-reader mise run mutants-diffreplace flash -> FlashDescription with Default::default(), which cannot compile becauseFlashDescriptionhas noDefault— a structural removal, not a survivor. All 78 are intag.rs,writer.rsanddescribe.rs.Beyond the gates, the transcription was checked three ways against the vendored PDF, since a
hand-transcribed table is the one thing a test cannot falsify on its own:
tests/oracle.rsand recorded inSTATUS.mdTypeandCount, against the per-tagTag = / Type = / Count =blocks the spec states beside each definition (the tables were transcribed from the summary tables, so this is an independent source)GainControl, fixed in its own commit — and no othersdescribe.rs, against the specification's own textThe last three were one-off cross-checks run against
pdftotextoutput ofreferences/exif/exif-3.0-dc-008-translation-2023.pdf; they are not committed, because they wouldput a
pdftotextdependency in the test tier for a check that only has to hold at transcriptiontime. The committed guards against later drift are the exiv2 name differential, the
only_the_non_dc008_tags_claim_no_field_typeandexactly_the_enumerated_tags_have_a_tableboundary tests, and the
GainControlpin.Run in a nested worktree.
mise run fmt/fmt-checkneed the
__CARGO_TEST_ROOTprefix in a nested worktree — cargo otherwise walks past the worktreeroot to the primary checkout's manifests and the task exits 101 on an untouched tree. That is an
environment artefact; no manifest was changed to work around it. Workspace-wide gates ran inside
systemd-run --user --scope --slice=agents.slice -p MemoryMax=16G -p MemorySwapMax=0withCARGO_BUILD_JOBS=2andulimit -v, per the run's resource rules.No failure in this run was caused by this branch. The two rustdoc warnings are classified
pre-existing on the evidence that they sit on lines this diff does not touch —
reader.rs:35and
stream.rs:7are unchanged from the base — rather than on a separate gate run of the base.No pre-existing failure was observed on the base.
Round 2 — review repairs
An independent review of this branch confirmed all six of the claims above and raised two Medium
findings and four Lows. Every one is repaired in the two commits
284c45d0andae353b69. Thebase branch had advanced and was merged again — twice, as it kept moving during this round:
first
ff6b5711/8c4d3d10(both inmaker_note.rs, neither touching this diff), then3242ea1b/44594fa1/ba0a139f, which do overlap this diff inREADME.mdandlib.rs. Thatsecond merge (
61d5934d) auto-merged with no conflicts — the two sides edit disjoint paragraphs,the base's in the reader/report prose and this branch's in the catalogue and write-door prose — and
every gate below was re-run at the final head,
8df62326, which is what these resultsdescribe.
Two further commits followed the review repairs, both correcting claims this round had itself
introduced — the same defect class F2 named, caught by re-reading the new text against the spec:
2c15d96f— the rewritten drift-guard doc asserted that the 39 tags with a table here are"exactly the tags for which CIPA DC-008 prints a value table". False in both directions.
Printing a table is not sufficient: §4.6.7.1.1 gives
GPSVersionID2.4.0.0 = Version 2.4 / Other = reserved, §4.6.6.1.2 givesFlashpixVersion0100 = Flashpix Format Version 1.0 / Other = reserved, §4.6.5.1.12 givesYCbCrSubSampling[2, 1] = YCbCr4:2:2 / [2, 2] = YCbCr4:2:0, and §4.6.8.1.1 givesInteroperabilityIndex"R98"/"THM"/"R03"— none ascalar code a
(u32, &str)row can carry. Nor is it necessary: §4.6.6.7.28 prints no table forFocalPlaneResolutionUnit, defining it as "the same as the ResolutionUnit", and it shares thatarm. The guard now states the rule the code implements — a single scalar code, per value or per
ComponentsConfigurationelement — and names the tags that decide the boundary. The 39 names itpins are unchanged and remain correct. The same commit also drops a false reassurance on
ExifTag(#[non_exhaustive]blocks exhaustive matching, notExifTag::Make as u32on afieldless enum, so "nothing may cast it" was wrong; the warning stays, the reasoning is
corrected) and narrows the README's pointer-drop claim to its mechanism —
writer.rsremoveseach pointer only from its home directory, so one hand-set elsewhere survives as an ordinary
field — and enumerates the lenient doors as the nine that admit a field, with
set_thumbnailnoted separately as admitting unvalidated bytes.8df62326—2c15d96fleft the guard doc andSTATUS.mdeach saying "Four tags whose sectionsprint one are excluded" above a list of five (Flash,
GPSVersionID,FlashpixVersion,YCbCrSubSampling,InteroperabilityIndex); the module docs had it right by counting Flashseparately. Corrected to five.
CARGO_BUILD_JOBS=2 cargo test -p gamut-exif --all-featuresgolden.rs,oracle.rs,report.rs,streaming.rs, doctests). The rise from 117 is the base's own new report tests arriving with the second merge.CARGO_BUILD_JOBS=2 cargo clippy -p gamut-exif --all-targets --all-features -- -D warnings__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmtthenmise run fmt-checkmise run check-testsconvco check origin/feat/419-exif-streaming-reader..HEADmise run lint(workspace,--all-targets --all-features -D warnings)warning:line is cargo advising that the transitive dependencyproc-macro-error2 v2.0.1will be rejected by a future rustc; pre-existing and unrelated.mise run test(workspace,--all-features)exiv2_knows_every_catalogued_tag_by_the_name_gamut_gives_it, so the serialised call sites still hold under the full suite.GAMUT_MUTANTS_BASE=origin/feat/419-exif-streaming-reader mise run mutants-diffmutants.out/missed.txtis empty; the unviable one is againreplace flash -> FlashDescription with Default::default(), which cannot compile.cargo doc -p gamut-exif --no-deps,--all-featuresand--no-default-features61d5934d. Earlier in this round it reported 2 warnings (reader.rs:10,reader.rs:35), both shown pre-existing on the base —git diff <base>..HEAD -- src/reader.rswas empty and both lines were present at the base — and the base's ownba0a139fhas since fixed them. This branch's edits added no rustdoc warning at any point. Notemise run lintis clippy only, so rustdoc is not a repository gate; it was run deliberately because this round is mostly documentation.mise run check-release-depsandmise run check-ffi-featureswere not re-run in round 2: noCargo.tomlwas touched, which is the condition the run's contract attaches them to. Their round-1results above still stand at this head.
No failure observed in round 2 was caused by this branch. Workspace gates ran inside
systemd-run --user --scope --slice=agents.slice -p MemoryMax=16G -p MemorySwapMax=0withCARGO_BUILD_JOBS=2andulimit -v, per the run's resource rules.What the review found, and what was done
describereturnedNonefor nine GPS reference tags DC-008 does enumerate (GPSLatitudeRef,GPSLongitudeRef,GPSSpeedRef,GPSTrackRef,GPSImgDirectionRef,GPSDestLatitudeRef,GPSDestLongitudeRef,GPSDestBearingRef,GPSDestDistanceRef), while the module documentsNoneas "the spec reserves that code" — so the crate stated something false about the specification.GPSStatusandGPSMeasureModeas the character-coded tags — the sentence that made this an oversight rather than a scoping choice — now names all eleven.exactly_the_enumerated_tags_have_a_tablewas documented as pinning "exactly which tags CIPA DC-008 enumerates" but pinned the 30 names the table happened to hold, so it asserted a false proposition and blocked F1's fix.README.mdclaimed "every tag CIPA DC-008 defines", overstating by three: §4.6.3.1.1/.2.1/.3.1 give the Exif, GPS Info and Interoperability IFD pointers their own Tag/Type/Count blocks.tag.rs's precise wording (the five tables), and both README andtag.rsnow say why the three pointer tags stay uncatalogued.lib.rsandwriter.rseach named one lenient write door where six are public.lib.rs:26claimed every catalogued tag carries a spec-mandated type and count, false for the nine non-DC-008 rows.tag.rs's statement.ExifTagvariants silently changed implicit discriminant.tag_id()is the on-disk identity, and the enum is#[non_exhaustive], repr-less and reached by nogamut-ffientry point.mise run testwas still running.The review also reproduced the round-1 mutation run exactly (78 mutants, 77 caught, 1 unviable,
missed.txtempty) and verified 127/127 description strings and 10/10 flash strings verbatimagainst the spec, so none of the round-1 transcription needed redoing.
One test was added
a_reference_tag_letter_means_what_its_own_section_says. A missing match arm is not a mutant, soneither the structural checks nor the mutation gate could have seen F1, and neither would see its
recurrence. The class that replaces them: these nine tags reuse the same few letters for unrelated
meanings —
'M'is a magnetic direction, miles per hour, or miles;'N'is north latitude, knots,or nautical miles — so an arm that swallowed one tag into a sibling's group would answer plausibly
and wrongly while every existing assertion still held (the domains stay the same size, ascending
and non-empty either way).
Risks and rollout
ExifTagis#[non_exhaustive], so the seven new variants are a minorchange.
field_types/component_count/TagCount,check_tag/set_tag_checked/TagConstraintError, and the wholedescribemodule are new surface; nothing existing changedshape.
Exif::parse,Exif::set_tagandExifWriterarebyte-for-byte the same. The only new refusal is one a caller must opt into by calling
set_tag_checked.ExifTag::ALLorder.Gamma(0xA500) moved to its sorted position at the end of the Exifblock.
ALLis documented as "in declaration order", so this is not a contract change, but aconsumer that hard-coded an index would notice.
describeis off by default and no workspace crate enablesgamut-exif/full, so no existingbuild grows.
TagConstraintErroris a second error type besideExifError, following theprecedent of
GpsConversionError.and the seven rows are separate commits in that order.
Issue
Refs #417. Not
Closes: the issue's headline is exiv2's 408-tag count, and the tags beyondCIPA DC-008 are filed as #532. The one piece of the stated acceptance that could not be met is
filed as #533.
Exifdatum::print()so a rendered-value differential ispossible
(found by this branch; worked around here at the call site)
describefeature so the umbrella can reach it #543 — gamut-metadata / gamut: forward gamut-exif'sdescribefeature so the umbrella can reachit (filed in round 2 under decision 8)
Type/Countcolumns from CIPA DC-008(filed in round 2 under decision 11)
Decisions taken
No human approved this plan. This is an unattended automation run; the record below is what a human
reads afterwards. Decisions 1–4 are the record this lane was given; 5–8 were appended by the lane
and are marked.
Unresolved review notes
tooling/exiv2-oracle/src/shim.cppandsrc/lib.rs, which are outside this lane's manifest —about fifteen lines of C++ exposing
Exiv2::Exifdatum::print()plus a safe Rust wrapper, in adev-only crate with no shipped surface. The lane did not widen its manifest; see decision 5 and
issue tooling/exiv2-oracle: expose Exifdatum::print() so a rendered-value differential is possible #533.
tests/oracle.rstripled the number of test threads calling exiv2 at once and produced a nondeterministic SIGSEGV
in CI's mutation baseline. This branch serialises the calls in the test file; the guard belongs
in
tooling/exiv2-oraclebeside the one its XMP half already takes, which is outside this lane'smanifest and is filed as tooling/exiv2-oracle: EXIF entry points are unguarded while the XMP ones are serialised #536. Nobody has captured a faulting stack trace, so the mechanism is
inferred from the shard-to-shard nondeterminism rather than observed directly.
gamut_exif::describenames both a module and a function. The module ispub, and thecrate root also re-exports the function out of it, so
gamut_exif::describe(tag, code)andgamut_exif::describe::describe(tag, code)both work — they live in different namespaces, sothis compiles and rustdoc renders both. It keeps the ergonomic call the README shows and matches
how every other module's items are re-exported at the root, but it is a wart, and dropping the
root re-export is the alternative.
describeis not surfaced through thegamutumbrella's feature table (crates/gamut/Cargo.tomlis outside the manifest, and
check-ffi-featurespasses without it). A consumer reaches it bydepending on
gamut-exifdirectly. Worth a follow-up if the umbrella should expose it.Round 2
cargo doc -p gamut-exifreportedreader.rs:10(public docs linking the privatecrate::stream) andreader.rs:35(unresolvedExifError::MissingMarker). Both were shown to belong to PR feat(exif): streaming ReadAt entry point + a report of what a lenient parse dropped #522'sdiff rather than this one —
git diff <base>..HEAD -- crates/gamut-exif/src/reader.rswas emptyand both lines were present at the base — so they were reported rather than repaired here, since
putting the fix in this diff would have placed a change in the wrong pull request. The base's
ba0a139ffixed them, andcargo docis clean at61d5934d. Resolved; recorded because thereasoning is what a later reader needs, not the outcome.
describetables arehand-transcribed like the rest, verified against the vendored spec text at transcription time and
guarded structurally afterwards. tooling/exiv2-oracle: expose Exifdatum::print() so a rendered-value differential is possible #533 remains the durable answer for description strings; nothing
covers
Type/Count, which is why gamut-exif: nothing in CI re-derives the per-tag Type/Count columns from CIPA DC-008 #544 was filed.describefeature so the umbrella can reach it #543 and stated plainly inlib.rs,README.mdandSTATUS.md: only a directgamut-exifdependent can enabledescribe.Round 3
Three Low review findings, all of them sentences that described the code's basis more strongly than
the code supports, plus one correction to a filed issue. One commit,
docs(exif): state the rule that selects the described tags; the diff is comment- and Markdown-only (git diff -U0 -- crates/gamut-exif/srchas no changed line that is not a///,//!or//line).STATUS.mdwere false in both directions. They said "thethirty-nine tags for which CIPA DC-008 prints a value table are exactly the tags that have one
here". The spec prints a table for
Flash§4.6.6.7.21,GPSVersionID§4.6.7.1.1,FlashpixVersion§4.6.6.1.2,YCbCrSubSampling§4.6.5.1.12 andInteroperabilityIndex§4.6.8.1.1, none of which has an arm, and prints none for
FocalPlaneResolutionUnit§4.6.6.7.28,which does. The number 39 was right; the rule behind it was unstated. Both places now state the
rule the code implements: a tag has a table exactly when DC-008 fixes the meaning of a single
scalar code — one integer, or one ASCII character — that the value carries on its own, or, for
ComponentsConfiguration, that each of its four elements carries on its own. The fourstructurally non-scalar tags are named as exclusions with the reason each is one (a bitfield; two
fixed multi-byte versions; a pair whose meaning belongs to its elements jointly; multi-character
ASCII codes), and
FocalPlaneResolutionUnitis named as the deliberate exception, admitted withno table of its own because §4.6.6.7.28 defines it as "the same as the ResolutionUnit"
(§4.6.5.1.11) rather than restating the values. The pinned set is unchanged: the rule was
restated to match the code, not the code relaxed to admit the four.
said
ExifTag"is#[non_exhaustive], carries norepr, and is reached by nogamut-ffientrypoint, so nothing may cast it to an integer". Only the last clause is load-bearing:
#[non_exhaustive]blocks exhaustive matching, notason a fieldless enum, and an externalcrate can and does cast it. The note now says plainly that nothing stops a cast, that the
discriminants are not stable, and that callers must not cast, persist or transmit them. The
warning is kept; the false reassurance is gone.
GPS pointers from the 0th IFD and the Interoperability pointer from the Exif sub-IFD — it keys on
each pointer's home directory — so one hand-set into any other directory survives as an ordinary
field that no accessor reads. The README claimed it "drops any that were hand-set"; it now states
the directory-keyed mechanism.
tag.rs's narrower claim was already sound and is unchanged.field; there are nine and all nine are now named (
set_tag,set,image_mut,exif_ifd_mut,gps_ifd_mut,interop_ifd_mut,set_exif_ifd,set_gps_ifd,set_interop_ifd), where sixwere named and three left to "and their siblings".
set_thumbnailis called out separately assitting outside that boundary: it admits bytes rather than a field, and does not check that they
are a JPEG.
tests/tag_names.rs. Thereis no such file — the check landed in
crates/gamut-exif/tests/oracle.rs. Everything else ingamut-exif: nothing in CI re-derives the per-tag Type/Count columns from CIPA DC-008 #544 is accurate. Recorded here because this run does not edit existing issues.
Follow-ups filed (both API additions on a crate already at a stable version, so out of scope
here and additive):
describekeys on #553 — a helper that takes a two-byte ASCII reference value and yields the codedescribekeyson, so every caller need not strip the NUL itself.
Default =values CIPA DC-008 prints, whichdescribed_valuescurrentlydrops and a consumer rendering the domain as a menu would want.
Validation for this round, verbatim, at
2c15d96f:cargo test -p gamut-exif --all-featuresRUSTDOCFLAGS="-D warnings" cargo doc -p gamut-exif --no-deps --all-features__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmt-checkmise run check-testsmise run check-commitsconvco check origin/feat/419-exif-streaming-reader..HEADmise run mutants-diffwas not run and is not applicable: the round's diff contains noexecutable line, so mutation testing has nothing in it to see.
mise run lintandmise run testwere not re-run either — no compiled behaviour changed, and the crate's own suite plus a
warnings-denied rustdoc build cover the doc comments this round touches. Earlier rounds' full-gate
results stand above and are not re-claimed here.
Residual risk. The rule now written down is a description of the pinned set, checked by
reading the six named sections of the vendored spec; nothing mechanically derives the set from the
spec, so a future tag whose section prints a table could be added or omitted against the rule and
only a human reading both would notice. That is the same class of gap as #544 and #533, narrowed
but not closed.
Round 3 addendum — final head
The Round 3 validation table above was recorded at
2c15d96f.8df62326(the five-versus-fourcount fix) landed on top of it, and every one of those gates was re-run at
8df62326with the sameoutcome:
8df62326cargo test -p gamut-exif --all-featuresRUSTDOCFLAGS="-D warnings" cargo doc -p gamut-exif --no-deps --all-features__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmt-checkmise run check-testsmise run check-commitsThe exclusion count defect
8df62326fixes was introduced by2c15d96fin this round and caughtbefore merge; it is recorded here rather than silently corrected, because "the new text asserts a
count the list beside it contradicts" is exactly the failure mode Round 3 set out to remove.