TIKA-4777: Expose presentation start of delayed QuickTime timed metadata tracks

[dschmidt]

JIRA: https://issues.apache.org/jira/browse/TIKA-4777

Builds on the QuickTime metadata support from #2935 (TIKA-2861, now merged).

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. Following review, this now emits a single dedicated property instead of the earlier per-key suffix scheme: quicktime:still-image-time, the presentation start in microseconds of a single-sample mebx track declaring com.apple.quicktime.still-image-time. An undelayed track yields 0, so "the still is the first frame" (0) stays distinguishable from "not a Live Photo" (absent). Other mebx keys produce no output.

Apple Live Photo videos mark the moment the paired still image was captured as the single one-tick sample of that 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).

The quicktime: namespace follows ExifTool, exiv2 and metadata-extractor, which all spell out QuickTime for this group; the keys are container-level and appear identically in audio-only files, so a video: prefix would be misleading. The unit stays out of the property name, matching xmpDM:duration, geo:alt and audio:bitrate.

Verified against a real Live Photo video (iPhone 15 Pro, iOS 18.5): quicktime:still-image-time = 1233333 (1.2333 s, matching the empty edit of 740/600 s); the undelayed video-orientation and live-photo-info tracks produce nothing.

Testing: testMP4_QuickTimeMetadata.mov asserts the emitted value plus the negative cases (foreign keys, non-leading empty edit, multi-sample track); the new testMP4_StillImageTimeZero.mov covers the declared-but-undelayed track (version 1 edit list with only a media edit) yielding 0.

[tballison]

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.

[dschmidt]

Thanks, that is remarkably thorough feedback, and almost all of it was right.
Addressed in 315e01a:

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?

[tballison]

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.

[dschmidt]

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.

[tballison]

LOL.. dedicated new property. I prefer "qt:" as the namespace, but I defer to fellow humans in the conversation.

[dschmidt]

I feel qt: is a bit unspecific? How about quicktime: or video:qt:?

[dschmidt]

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.

[dschmidt]

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.

[tballison]

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.

[dschmidt]

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.