You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by "dschmidt (via GitHub)" <gi...@apache.org> on 2026/07/09 10:51:27 UTC
[PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
dschmidt opened a new pull request, #2936:
URL: https://github.com/apache/tika/pull/2936
JIRA: https://issues.apache.org/jira/browse/TIKA-4777
Stacked on #2935 (TIKA-2861), only the last commit is new here. Marked as
draft until that PR is merged; after a rebase only the TIKA-4777 commit
will remain.
QuickTime timed metadata tracks (`mebx` sample descriptions, ISO 14496-12
boxed metadata) declare their key names in the file, and a track's
presentation start is defined by its edit list: a leading empty edit
delays the media by its duration. This parses the mebx key declarations
(keys/keyd) and emits, for every declared key of a delayed track, the
presentation start in microseconds as `<key name>.track-start-us`.
Undelayed tracks are not reported, their start carries no information.
No key names are hardcoded. The main use case are Apple Live Photos,
which mark the moment the paired still image was captured as the single
one-tick sample of the `com.apple.quicktime.still-image-time` track,
shifted there by an empty edit. The sample value itself is a constant -1
marker, so the moov boxes alone are sufficient and mdat is never read.
Apple's own LivePhotosKit JS player derives its `photoTime` the same way
(it parses trak/elst/mebx client-side).
Verified against a real Live Photo video (iPhone 15 Pro, iOS 18.5):
`com.apple.quicktime.still-image-time.track-start-us = 1233333` (1.2333s,
matching the empty edit of 740/600s), likewise the transform keys
declared by the same track; the undelayed video-orientation and
live-photo-info tracks are not reported.
Testing: the crafted `testMP4_QuickTimeMetadata.mov` fixture gains a
timed metadata track with a proper mebx keys/keyd declaration and an
empty edit; `MP4ParserTest` asserts the emitted track start.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4938303696
How about this? from my agent...
```
Q2 (absence vs 0): once it's a property, emit 0
With a dedicated property, emit 0 for a declared-but-undelayed single-sample still-image-time track. Then 0 =
"still is the first frame," absent = "not a Live Photo," which is precisely the disambiguation a consumer wants.
dschmidt's only hesitation — "noise on the other keys" — was an artifact of the per-key scheme. Under one
property there are no other keys, so it evaporates. Q1 and Q2 are the same decision: pick the property and 0
becomes correct.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4934951662
Feedback from claude:
```
The feature is worth having and the byte-slicing is genuinely bounds-safe (I traced every offset; no OOB, no infinite loops, entry counts can't run away). But three things
stand out: about half the diff re-implements traversal that metadata-extractor already does, there are two real bugs in the edit-list logic, and the metadata key shape
deserves a conversation before it becomes API.
1. Most of the plumbing is redundant (verified, not guessed)
Mp4MetaHandler extends Mp4MediaHandler, whose shouldAcceptContainer already accepts minf and stbl, and whose shouldAcceptBox already accepts stsd.
Mp4MetaHandler.processSampleDescription(SequentialReader) is an empty override waiting to be filled in.
I confirmed this by running the stock handler plus a one-line hdlr injection against the PR's own fixture:
>>> injecting TracingMetaHandler at hdlr/meta
MetaHandler.shouldAcceptContainer(minf) = true
MetaHandler.shouldAcceptContainer(stbl) = true
MetaHandler.shouldAcceptBox(stsd) = true
*** Mp4MetaHandler.processSampleDescription CALLED ***
first 16 bytes of stsd payload: 0000000000000001000000506d656278
entry type at +12: 'mebx'
So instead of the hdlr hijack that returns this and suppresses Mp4MetaHandler entirely, plus the global minf/stbl/stsd acceptance in TikaMp4BoxHandler, the PR could return
a TikaMp4MetaHandler extends Mp4MetaHandler from the hdlr branch (passing in emptyEditDuration and the movie timescale) and override processSampleDescription. That would:
- keep Mp4MetaDirectory alive rather than silently deleting it from the extracted directories,
- stop TikaMp4BoxHandler from descending into minf/stbl and slurping the stsd payload of every track whose handler type isn't soun/vide/hint/text/meta (sbtl, clcp, tmcd,
…), which it now does for no benefit,
- delete shouldAcceptContainer's minf/stbl cases and shouldAcceptBox's stsd case.
Only edts (container) and elst (box) genuinely need adding to TikaMp4BoxHandler — I confirmed the stock handler returns false for both.
Separately, processMovieTimescale is dead weight: the base processMovieHeader already stores the movie timescale, and directory.getLongObject(Mp4Directory.TAG_TIME_SCALE)
returns 600 on this exact fixture. Ten lines and a field can go.
To be fair about the Mp4MetaDirectory deletion: I checked the tag IDs, and Mp4MetaDirectory only ever holds TAG_CREATION_TIME=101, TAG_MODIFICATION_TIME=102,
TAG_LANGUAGE_CODE=104, none of which collide with the 0x01xx tags processActualMp4Directory reads. So there's no visible regression today. It's a latent one.
2. Two correctness bugs in processEditList
The javadoc says "leading empty edit," but the loop scans every entry and takes the first mediaTime == -1 wherever it appears:
- Edit list [(dur=100, mt=0), (dur=740, mt=-1), (dur=1, mt=0)] — the track presents media starting at movie time 0, so the start is 0. The code reports 1233333.
- Two consecutive leading empty edits [(740, -1), (300, -1), (1, 0)] — the true start is 1040 ticks. The code reports 740.
The fix is to only honor entry 0, and to sum consecutive leading -1 entries. Neither shape appears in Apple's own files, but the code doesn't say that and nothing stops a
fuzzer or a third-party muxer.
Related, and cheaper to fix than to argue about: emptyEditDuration * 1_000_000L overflows for a version-1 elst with a large segment_duration. A emptyEditDuration >
Long.MAX_VALUE / 1_000_000 guard costs one line.
Also, resetting emptyEditDuration on tkhd assumes tkhd precedes edts. It's mandated, but resetting on the trak container instead (processContainer routes through
processBox("trak", null, …)) is free and order-independent.
3. The design question I'd want settled before merge
<key>.track-start-us attaches a track-level fact to every key name the track declares. The PR body confirms this in practice: on the real iPhone Live Photo, the
still-image-time track also declares transform keys, and all of them get a .track-start-us sibling.
For still-image-time the statement is meaningful only because that track has exactly one one-tick sample, so track-start is the sample's presentation time. The code never
checks the sample count — it filters on "is delayed," which correlates with "single sample" in Apple's output but isn't the invariant. Note the JIRA description explicitly
lists stts among the boxes to correlate; the PR doesn't read it. A delayed multi-sample mebx track would get <key>.track-start-us on keys whose values are timestamped all
over the track.
Two smaller shape questions:
Namespace. com.apple.quicktime.still-image-time.track-start-us is a Tika-invented suffix grafted into Apple's namespace. It reads like a key Apple defined. It doesn't
collide with reserved Tika keys (the suffix makes that impossible), so it's not a trust-boundary problem in the TIKA-4769 sense — but it's the kind of thing that's hard to
rename later.
Absence is ambiguous. Suppressing undelayed tracks means "no still-image-time.track-start-us" can't distinguish "still is the first frame" from "not a Live Photo."
Emitting 0 for a declared-but-undelayed still-image-time track would be unambiguous, at the cost of noise on the other keys — which loops back to the per-key question.
4. Test gaps
The fixture is a hand-built .mov whose minf contains only stbl and whose stbl contains only stsd. Real QuickTime metadata tracks have gmhd, a data-reference hdlr
(dhlr/alis), and dinf inside minf, plus stts/stsc/stsz/stco inside stbl. The test therefore never exercises the traversal as it will actually run — in particular that
inner hdlr, which under the PR's approach now reaches super.processBox and gets handler-type-dispatched. It works (I checked: alis falls through), but the test wouldn't
catch it if it stopped working.
Nothing covers: an undelayed meta track producing no key (asserted in the PR body, untested), a version-1 elst, a non-leading empty edit, or a video track with a leading
empty edit followed by a meta track without one (the state-leak path).
Nits
- The shouldAcceptContainer comment says timed metadata tracks have handler type mdta. They have meta — mdta is the moov/meta/hdlr metadata handler. The code's own hdlr
check disagrees with the comment.
- readInt64 is defined between boxType and addLocation, far from its only caller.
- tikaMetadata.set(...) here vs tikaMetadata.add(...) on the ilst path — two delayed tracks declaring the same key silently overwrite.
- The fixture change also adds mvhd and a trak to a file that previously had neither, so testQuickTimeMetadataKeys now additionally produces dcterms:created,
xmpDM:duration, and xmpDM:audioSampleRate=600 (the last being a pre-existing MP4Parser quirk — it maps movie timescale to audio sample rate — not this PR's fault).
Existing assertions still pass; just noting the fixture is no longer inert.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4978296729
Went with `quicktime:` after looking at precedent: ExifTool, exiv2 and metadata-extractor all spell out "QuickTime" for this group, none abbreviate to qt, and the abbreviation reads like the C++ framework to me. `video:` would be wrong anyway since these keys live at container level and audio-only .m4a files carry them too (Voice Memos write QuickTime atoms). So the property is `quicktime:still-image-time`, emitted (including 0) for a single-sample still-image-time track, absent otherwise, exactly the shape you described.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4980578460
From my agent:
```
One real finding (DoS/robustness): TikaMp4MetaHandler.processSampleDescription does getBytes((int) entrySize - 8)
where entrySize is a uint32 only guarded by < 16. A crafted mebx stsd with entrySize = 0xFFFFFFFF gives (int)-1
- 8 = -9; getBytes's bounds check passes for a negative count and hits new byte[-9] → NegativeArraySizeException,
which escapes both Mp4Reader's catch (IOException) and MP4Parser's catch (RuntimeSAXException), out of parse().
Reproduced by patching the one-track fixture's entry size: threw java.lang.NegativeArraySizeException: -9
(pristine control parses clean). New to this PR (stock Mp4MetaHandler.processSampleDescription is empty).
One-line fix:
if (entrySize < 16 || entrySize - 8 > reader.available()) { return; }
No other DoS/OOM/loop concerns: all loops are bounded or strictly advance (every pos += size has a size < 8 →
break guard); getBytes bounds-checks before allocating so oversized positive counts throw EOF without allocating;
payloads are bounded by Mp4Reader's box-size limits; keyNames growth is bounded.
Minor (non-blocking):
- The trak-container emptyEditDuration reset is correct but not actually exercised — every meta track after track
1 has a foreign key or >1 sample, so a leaked value never reaches an emit. A real regression test needs a
single-sample still-image-time track with no leading empty edit placed after a delayed track, asserting 0.
- hdlr branch keys on handler_type == 'meta'; a (non-real-world) moov-level meta/hdlr with type 'meta' would
bypass keys/ilst. Latent only.
- set vs add on STILL_IMAGE_TIME: fine now that it's one scalar (last-writer-wins), just noting.
- ~4 consecutive blank lines after processEditList.
quicktime:still-image-time shape is right: internalReal is required for set(Property, long) and serializes to
exactly "1233333"/"0"; movie-timescale divisor is the correct unit for elst segment_duration per ISO 14496-12.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4945505603
LOL.. dedicated new property. I prefer "qt:" as the namespace, but I defer to fellow humans in the conversation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4985130422
Good catch on the entry size, that one was real: without the guard the crafted stsd escapes parse() as a NegativeArraySizeException (the new regression test reproduces exactly that when the guard is removed). Entry sizes beyond the remaining payload are now rejected as malformed. I also took the test gap point: the zero fixture now places a delayed foreign-key track before the undelayed still-image-time track, so an empty edit duration leaking between tracks would surface as a non-zero value in the existing assertion. Blank lines are gone too. The moov-level hdlr point I left alone: it is latent as you say, real-world container-level metadata declares 'mdta' there rather than 'meta', so guarding against it felt speculative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4935396937
Thanks, that is remarkably thorough feedback, and almost all of it was right.
Addressed in 315e01a30:
1. **Plumbing**: reworked as suggested. The hdlr branch now returns a
`TikaMp4MetaHandler extends Mp4MetaHandler` that fills in the empty
`processSampleDescription`/`processTimeToSample` overrides. The global
minf/stbl/stsd acceptance and the movie-timescale duplication are gone,
`Mp4MetaDirectory` stays alive; only edts/elst remain in
`TikaMp4BoxHandler`.
2. **Edit list**: both bugs fixed: only leading empty edits count, summed,
and the scan stops at the first normal entry. Overflow guard added, and
the per-track state resets on the trak container instead of tkhd.
3. **Sample count**: stts is now read and only single-sample tracks are
reported, which is the actual invariant behind "track start = still
moment" (previously the delay filter only correlated with it).
4. **Tests**: the fixture gained realistic track plumbing (gmhd, dhlr/alis
data handler, dinf/dref, full sample tables) plus tracks covering a
version-1 elst, a non-leading empty edit directly after the delayed
track (state-leak probe), and a delayed multi-sample track. set() is now
add(), and the comment/placement nits are fixed. I re-verified against a
real iPhone 15 Pro Live Photo: output unchanged.
Two shape questions I deliberately did not touch and would like your call
on before merge:
- **Namespace**: keep `<apple key>.track-start-us`, or move to a Tika-owned
scheme? I agree it is hard to rename later, so happy to change it now.
- **Absence vs 0**: with the single-sample invariant in place, emitting 0
for an undelayed single-sample track would make "still is the first
frame" distinguishable from "not a Live Photo" without adding noise on
multi-sample keys. Which do you prefer?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4944135635
My agent wants to know:
```
Before I touch the key shape, I want to make sure I read this right: the
quoted Q2 section presupposes the Q1 answer, so let me confirm the intended
shape. Is it: drop the per-key <key>.track-start-us scheme entirely, and
instead define a single dedicated property that is emitted (including 0)
whenever a single-sample mebx track declares
com.apple.quicktime.still-image-time?
If so: did the Q1 section of your agent's analysis suggest a concrete
property name and namespace (something like quicktime:stillImageTimeUs)?
I would rather implement exactly what you have in mind than guess on a name
that is hard to rename later.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4978408857
Reworked as discussed: the per-key `.track-start-us` suffix is gone, replaced by the dedicated `quicktime:still-image-time` property (microseconds, 0 for a declared but undelayed single-sample track, absent otherwise). Re-verified against the real iPhone Live Photo video: `quicktime:still-image-time = 1233333`. PR description is updated accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "dschmidt (via GitHub)" <gi...@apache.org>.
dschmidt commented on PR #2936:
URL: https://github.com/apache/tika/pull/2936#issuecomment-4968690854
I feel qt: is a bit unspecific? How about quicktime: or video:qt:?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks [tika]
Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison merged PR #2936:
URL: https://github.com/apache/tika/pull/2936
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tika.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org