Skip to content

[format][python] Persist keyframe indexes for efficient random video frame reads - #9831

Open
XiaoHongbo-Hope wants to merge 8 commits into
apache:masterfrom
XiaoHongbo-Hope:codex/video-frame-mapping-index
Open

XiaoHongbo-Hope wants to merge 8 commits into
apache:masterfrom
XiaoHongbo-Hope:codex/video-frame-mapping-index

Conversation

@XiaoHongbo-Hope

@XiaoHongbo-Hope XiaoHongbo-Hope commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Avoid full-video probing and scan amplification for cold random frame reads. PyPaimon now generates a seek index while ingesting supported MP4 videos. PaimonLeRobotDataset uses it to prefetch video metadata and requested GOP ranges, then serves any uncached decoder reads through Range GET.

Design

.video (in file order)
|-- Complete video payloads A, B, ...
|-- NEW: Video seek index blocks A, B, ...
|   |-- Video metadata ranges: (offset, length) pairs
|   `-- Compressed keyframe entries: (frame ordinal, PTS, packet byte position)
|-- Video payload lengths
|-- NEW: Video seek index block lengths
|-- Consecutive-row counts / video references / starting frame ordinals
`-- Footer
    |-- Five index byte lengths: 5 x 4 bytes
    |-- Magic: 4 bytes
    `-- Format version: 1 byte

Each video has one block-length entry; zero selects the existing scan fallback. All seek-index offsets are relative to the first byte of that encoded video.

Stored value Meaning Cold-read use
Video metadata offset and length Location and size of initialization metadata such as MP4 moov Initialize the container without probing the full video
Frame ordinal Zero-based frame number in presentation order Find the preceding keyframe for the requested frame
PTS Presentation timestamp Seek to the exact keyframe without assuming constant frame rate
Packet byte position Offset of the compressed keyframe packet Start the target GOP range read

GOP index and in-GOP frame index are derived from the requested ordinal and preceding keyframe, so the format stores entries only for keyframes. PTS keeps VFR exact, and packet position enables Range GET. Stream identity and time base remain in the encoded video.

Read and write paths

  • PyPaimon resolves local source lengths, inspects supported ISO BMFF videos, and persists metadata ranges plus keyframe ordinals, PTS values, and packet positions. Initialization ranges keep box headers but omit mdat, free, and skip bodies. Unsupported videos and environments without PyAV retain the scan fallback.
  • Supplied indexes are validated and preserved by add, add_video, add_videos, replace_video, rolling, and compaction. Conflicting indexes for one payload fail the write.
  • PaimonLeRobotDataset loads the index, prefetches initialization metadata and a bounded GOP window through FileIO.read_ranges_coalesced, and gives PyAV a seekable range-backed stream. Uncached decoder reads fetch actual payload bytes; no zero-filled temporary file or fixed retry heuristic is used.
  • Indexed default reads prefer the PyAV range-backed path and fall back to TorchCodec when PyAV is unavailable or cannot initialize. Explicit PyAV and TorchCodec selections keep their requested backend.
  • Java and Python validate indexes in bounded chunks and reject more than 65,536 metadata ranges or keyframes. Writers cap one block at 16 MiB and all buffered index blocks in one file at 64 MiB.
  • The unreleased version 1 container and descriptor layouts are updated in place.

Current open-source Lance comparison

The current open-source LeRobot-Lance stores each source MP4 and its seek metadata as one row in videos.lance:

Paimon seek metadata Lance column
Video metadata offset and length moov_offset, moov_size
Frame ordinal kf_indices
Packet byte position kf_positions
PTS Not stored separately; its reader assumes constant-frame-rate MP4

Paimon stores the metadata beside its payload in the same .video file and compresses the keyframe entries.

Benchmark

CPU-only, single-node ecs.g8i.xlarge (4 vCPU, 16 GiB) against same-region OSS internal endpoints. Each of 10 samples used a fresh decoder, HTTP session, and client byte cache; OSS service-side cache was not controlled. Both videos contained 20,000 frames.

Request Indexed latency p50 Scan fallback p50 Indexed / fallback GETs Indexed / fallback bytes
Single frame 133–172 ms 37.0–55.3 s 7 / 3,107–4,387 0.25 MiB / 97–137 MiB
Random batch 32 755–884 ms 37.3–55.2 s 69–72 / 3,139–4,420 1.56–1.71 MiB / 98–138 MiB

Index generation took 17.3–22.8 seconds per video. The 59.7–60.6 KiB indexes were 0.043–0.060% of video size. All decoded frame hashes matched. The paired run used 7fa244ccf1; an indexed rerun on f3bb9da803 measured 154–196 ms for one frame and 801–848 ms for batch 32 with unchanged hashes and GET counts.

Validation

  • Public LeRobot path: load_from_lerobot -> .video -> PaimonLeRobotDataset -> two-worker DataLoader; decoded B-frame MP4 output matched expected pixels. A middle-frame read from a 120-frame, multi-GOP video transferred fewer total bytes than the complete payloads, including repeated range requests and index reads.
  • Real codec loops: MPEG-4 Part 2, fragmented H.264, HEVC, two video streams, B-frames, VFR, and trailing moov; indexed output matched full PyAV decoding. The MPEG-4 test omits media prefetch to verify on-demand reads.
  • Local path and Blob.from_local ingestion both generated indexes; large free/skip bodies were excluded from initialization ranges.
  • Python: 370 passed, 36 skipped, and 159 subtests passed across format, index, video, LeRobot, table, rolling, and BLOB suites.
  • Java: 4 descriptor, 13 video-format, and 2 rolling/compaction tests passed.
  • Python 3.6 syntax, flake8, and git diff --check passed.

@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Support persisted exact video frame mappings [format][python] Support efficient exact video frame reads Sep 15, 2026
@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Support efficient exact video frame reads [format][python] Support video frame reads with persisted keyframe indexes Sep 17, 2026
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch 4 times, most recently from d8fffbd to 12e689e Compare September 19, 2026 09:15
@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Support video frame reads with persisted keyframe indexes [format][python] Persist keyframe indexes for efficient random video frame reads Sep 19, 2026
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch 8 times, most recently from 0db216c to 405600d Compare September 19, 2026 10:20
@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Persist keyframe indexes for efficient random video frame reads [format][python] Persist video keyframe indexes Sep 19, 2026
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch 3 times, most recently from a8b3b9f to 9f115c2 Compare September 19, 2026 11:00
@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Persist video keyframe indexes [format][python] Add video keyframe index format for efficient random reads Sep 19, 2026
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review September 19, 2026 13:26
descriptor = frame.keyframe_index_descriptor
if descriptor is None:
return b''
mapping = Blob.from_descriptor(blob.uri_reader, descriptor).to_data()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Avoid materializing unbounded keyframe indexes

This materializes the entire compressed index before the chunked validator runs, and _keyframe_indexes retains every block until the writer closes. BlobRef.to_data() issues a single read for the descriptor length, while rolling is checked only after add_element; therefore a valid or crafted large index can allocate beyond the worker heap despite the 64 KiB decompression chunks. The Java path has the same behavior in VideoFormatWriter.java. Please validate from a bounded stream into spillable storage, or enforce explicit per-index and cumulative limits, instead of materializing unbounded bytes.

@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as draft September 20, 2026 02:21
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review September 21, 2026 12:27
@JingsongLi

Copy link
Copy Markdown
Contributor

The format work is careful, and the bounded-validation issue from my earlier review appears addressed. However, this PR still does not deliver the claimed random-read benefit to a Paimon user: PaimonLeRobotDataset/PyAV neither generates indexes nor consumes them here, and the sparse decode is implemented only inside video_format_test.py. The normal LeRobot loader writes descriptors with no index (-1, 0), so ordinary writes continue to produce unindexed videos. This adds a persisted Java/Python format and descriptor change without an end-to-end producer/consumer path. Please bring back the format together with index generation and a real table-reader path that fetches fewer bytes for a frame, with a test through that public path. Closing this PR for now on end-to-end value grounds.

@JingsongLi JingsongLi closed this Sep 22, 2026
@XiaoHongbo-Hope

Copy link
Copy Markdown
Contributor Author

The format work is careful, and the bounded-validation issue from my earlier review appears addressed. However, this PR still does not deliver the claimed random-read benefit to a Paimon user: PaimonLeRobotDataset/PyAV neither generates indexes nor consumes them here, and the sparse decode is implemented only inside video_format_test.py. The normal LeRobot loader writes descriptors with no index (-1, 0), so ordinary writes continue to produce unindexed videos. This adds a persisted Java/Python format and descriptor change without an end-to-end producer/consumer path. Please bring back the format together with index generation and a real table-reader path that fetches fewer bytes for a frame, with a test through that public path. Closing this PR for now on end-to-end value grounds.

Thanks, I did the read related change in the previous commit of this PR, but removed later. I will add back.

@JingsongLi JingsongLi reopened this Sep 22, 2026
@XiaoHongbo-Hope XiaoHongbo-Hope changed the title [format][python] Add video keyframe index format for efficient random reads [format][python] Persist keyframe indexes for efficient random video frame reads Sep 22, 2026
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch from 101fc42 to 81ee157 Compare September 22, 2026 06:09

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The payload-relative sparse-index layout can be retained. These two issues affect indexed-read correctness and read amplification.

Comment on lines +1698 to +1699
container.seek(
anchor_pts, backward=False, any_frame=False, stream=stream)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Seek backward to the persisted keyframe anchor

backward=False can skip the indexed keyframe, but the loop below requires an exact match for anchor_pts. With PyAV 18.1.0, a valid 150-frame fragmented H.264 MP4 (movflags=frag_keyframe+empty_moov+default_base_moof) is automatically indexed successfully, yet all 150 indexed reads fail: seeking to the first anchor at PTS 1024 starts at the next GOP, PTS 8704. Ordinary HEVC MP4 also fails for several frame ranges. The same forward seek skips the anchor on the complete file, so fetching more bytes or retrying earlier anchors does not fix it; the unindexed reader returns the correct frames.

Please seek backward and decode until the stored anchor is found, and add real fMP4/HEVC regression coverage. Changing only this flag to backward=True made all 750 frame comparisons pass across closed/open-GOP H.264, fragmented H.264, and HEVC, without changing the stored fields.

Comment on lines +171 to +172
ranges.append((
offset, header_size if box_type == b"mdat" else size))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Exclude padding bodies from initialization ranges

Treating every non-mdat box as initialization data includes the entire body of free/skip boxes. These ranges are fetched for every uncached indexed read, so MP4 files with reserved space can retain almost full-file read amplification despite the sparse index. I reproduced this with a valid MP4 remuxed using FFmpeg's -moov_size 16777216: reading frame 75 requests 16,779,069 bytes out of a 16,785,845-byte file. Keeping only the padding box headers reduces that request to 4,329 bytes, and all 150 decoded frames still match full decoding.

Please preserve the headers needed for box traversal while excluding free/skip bodies from the persisted initialization ranges. Other boxes should be handled according to their decoding requirements rather than dropped indiscriminately.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new producer/consumer path addresses my earlier end-to-end concern: the writer now generates seek indexes and PaimonLeRobotDataset consumes them. I verified the public import -> persisted video -> two-worker DataLoader test on this head, together with the format/index tests (20 passed). There are two remaining issues: automatically indexed MPEG-4 Part 2 videos can fail reads that the scan path handles correctly, and the documented local path/Blob.from_local inputs silently bypass index generation. Details and reproduction conditions are in the inline comments.

Comment on lines +1668 to +1670
raise ValueError(
"Cannot decode video frames from persisted seek index."
) from last_error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep valid videos on a readable path when sparse seeking is unsupported

Automatic index generation accepts MPEG-4 Part 2 MP4s, but these retries do not always decode them. With PyAV 18.1.0, I encoded 60 frames using mpeg4, GOP 12 and 2 B frames, then ingested them through add_video(BlobDescriptor(uri, 0, length), rows). The writer persisted a valid 140-byte index, but cold reads of frames 36, 48 and 59 through VideoFrameCollator with the dataset decoder all reach this exception. The identical persisted payload and frame indices, with the index omitted, match full PyAV decoding exactly. Thus successful ingestion can now make a valid video unreadable through default/PyAV reads. Please retain the codec initialization data needed for these seeks, or decline index generation for unsupported cases so they keep the scan path, and add this regression case.

Comment on lines +173 to +176
mapping = VideoKeyframeIndex.inspect(
stream, frame.payload_descriptor.length).serialize()
except (ImportError, OSError, ValueError, EOFError):
return b''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Resolve unknown source lengths before inspecting the video

The documented add_video(path, rows) and add_video(Blob.from_local(path), rows) inputs carry BlobDescriptor.length == -1. Passing that sentinel to inspect makes _iso_bmff_metadata_ranges skip its box loop and raise; this handler silently stores an empty index. Copying the payload later discovers its length but never retries indexing. I verified the public API with the same supported H.264 MP4: path and Blob.from_local inputs produced 60 unindexed frame rows, while an explicit descriptor with the actual length produced a 133-byte index on every row. Please resolve the effective source length before inspection and cover both documented local-file inputs, so ordinary ingestion receives the advertised random-read benefit.

@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch from 81ee157 to 639907b Compare September 22, 2026 06:24
@JingsongLi

Copy link
Copy Markdown
Contributor

My concrete recommendation is to keep the current storage layout and replace the zero-filled sparse temporary file with a seekable file-like reader backed by range reads. Use the persisted index to prefetch likely ranges; when the decoder requests uncached bytes, fetch the actual bytes from the video payload. This makes decoding correctness independent of the fixed extra-GOP and two-keyframe retry heuristics.

Also apply the two inline fixes: exclude free/skip padding bodies from initialization ranges and seek with backward=True.

I do not see a demonstrated need to add DTS or GOP-length fields, or to redesign the on-disk layout.

@JingsongLi

Copy link
Copy Markdown
Contributor

Please add a reproducible end-to-end benchmark for the cold random-read claim. The current tests verify decoding correctness and that selected payload ranges cover fewer bytes than the full video. The byte-count assertion measures unique payload coverage; it excludes index reads and does not count repeated transfers. There is no indexed/unindexed latency or request-count comparison.

Use representative long videos in object storage, the same decoder, and identical random target frames with and without the persisted index. Start cold runs with fresh decoder/cache state and report:

  • End-to-end first-frame and random-batch latency (p50/p95).
  • Actual total bytes transferred and GET counts, including index, metadata, media, and retries.
  • Additional ingestion time and index size from generating the index.

This would quantify both the read benefit and the cost shifted to ingestion.

@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/video-frame-mapping-index branch from 639907b to 0a3a923 Compare September 22, 2026 06:45

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current head now has end-to-end value: normal video ingestion generates the persisted seek index, and PaimonLeRobotDataset consumes it through the public read path. The earlier missing producer/consumer path, unbounded index buffering, unknown local-source length, padding prefetch, and forward-seek issues are addressed in this revision.

I revalidated the format and reader behavior on cf35bae: Java VideoFrameDescriptorTest 4/4, VideoFileFormatTest 13/13, and BlobTableTest 49 passed/1 skipped; Python index/format tests 25/25 plus 16 subtests passed with PyAV 15.1.0. I also repeated the prior MPEG-4 Part 2 case with fresh decoders for individual frames 0, 1, 11, 12, 24, 36, 48, and 59: all matched full-file decoding. On a 195,793-byte video those reads transferred 44,946–88,889 bytes each, including repeated range reads. The PR's public import-to-DataLoader test covers the integrated path in CI; my local environment lacks Torch, so I did not rerun that specific test locally. I did not independently reproduce the object-store benchmark.

I found no additional code blocker in this review. The production gate is still red: Python 3.10–3.13 CI jobs fail and Flink 1 Common was cancelled. The Python 3.10 log shows 36 failures in native_commit_test.py (an unchanged file in this PR), so these may be a branch/base issue, but the failures and cancelled job need a green rerun or documented resolution before merge.

source = _RangeBackedVideo(
self._video_length(), self._read_video_ranges)
try:
source.prefetch(_merge_video_ranges(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Bound batch range prefetch by bytes

This prefetches the metadata plus every GOP window for all missing indices in one call, and _RangeBackedVideo._ensure keeps every returned body in _segments until the whole batch finishes. A shuffled batch that selects frames from many distant GOPs in one large, valid video can therefore materialize the sum of those windows, potentially approaching the whole video; multiple DataLoader workers multiply the peak. The 16/64 MiB limits only bound index blocks, not video ranges. Please decode groups in byte-bounded chunks or evict range segments instead of retaining the full batch working set.

ordinal = physicalVideoLengths.size();
physicalVideoLengths.add(length);
Blob keyframeIndex = VideoFrameDescriptor.keyframeIndexBlob(blob);
byte[] mapping = keyframeIndex == null ? new byte[0] : keyframeIndex.toData();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Apply the configured NULL policy to the index fetch

payloadWriter.write(element) honors blob-write-null-on-missing-file and blob-write-null-on-fetch-failure, but this independent toData() fetch bypasses both policies and the fetch metrics. If the payload range succeeds and the index range then returns 404/416, is truncated, or fails transiently, the writer aborts even when NULL fallback is enabled; the payload bytes have already been written by that point. Please fetch and validate the index through the same policy before writing the payload, append NULL when the configured fallback applies, and cover payload-success/index-failure for both options.

"Corrupt video file: negative keyframe index length."
)

keyframe_index_size = sum(keyframe_index_lengths)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Enforce the documented index-size limits while reading

The format specification limits each keyframe-index block to 16 MiB and all blocks in one file to 64 MiB, but this reader only rejects negative lengths or totals outside the file. VideoFrameCollator later passes the declared length to read_file_range and materializes the entire block before VideoKeyframeIndex.deserialize can validate it. A corrupt or forged .video file can therefore force an arbitrarily large range read and allocation. Please reject per-block and cumulative lengths above the documented limits in VideoFileMeta before exposing these descriptors, and mirror the check in the Java reader.

stream.seek(position)
mapping = VideoKeyframeIndex.inspect(
stream, payload_length).serialize()
except (ImportError, OSError, ValueError, EOFError):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Include PyAV FFmpeg errors in the scan fallback

The documented behavior is that videos which cannot be indexed use the scan fallback, but this handler does not cover every av.error.FFmpegError. For example, decoder-not-found and other FFmpeg failures may inherit LookupError or only FFmpegError, so they escape this tuple and abort ingestion instead of returning an empty index. The reader in this PR already adds av.error.FFmpegError dynamically in _video_decoder_fallback_errors; please apply the same exception family here and add a real PyAV regression test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants