JingsongLi commented on code in PR #9831:
URL: https://github.com/apache/paimon/pull/9831#discussion_r4068948224
##########
paimon-python/pypaimon/write/video_format_writer.py:
##########
@@ -115,27 +141,68 @@ def close(self) -> None:
self._run_first_frames
)
for index in (
- physical_index,
- run_length_index,
- run_reference_index,
- first_frame_index,
- ):
+ physical_index,
+ keyframe_length_index,
+ run_length_index,
+ run_reference_index,
+ first_frame_index):
self.output_stream.write(index)
- self.output_stream.write(struct.pack(
- '<IIIIIB',
+ footer = struct.pack(
+ '<IIIIIIB',
len(physical_index),
+ len(keyframe_length_index),
len(run_length_index),
len(run_reference_index),
len(first_frame_index),
self.FOOTER_MAGIC_NUMBER,
self.VERSION,
- ))
+ )
+ self.output_stream.write(footer)
if hasattr(self.output_stream, 'flush'):
self.output_stream.flush()
if hasattr(self.output_stream, 'close'):
self.output_stream.close()
self._closed = True
+ def _keyframe_index(self, blob, frame):
+ descriptor = frame.keyframe_index_descriptor
+ if descriptor is None:
+ stream = blob.new_input_stream()
+ try:
+ try:
+ mapping = VideoKeyframeIndex.inspect(
+ stream, frame.payload_descriptor.length).serialize()
+ except (ImportError, OSError, ValueError, EOFError):
+ return b''
Review Comment:
[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.
##########
paimon-python/pypaimon/multimodal/lerobot/dataset.py:
##########
@@ -1579,6 +1608,150 @@ def __getitem__(self, index):
) from error
raise IndexError("Video frame index %d is out of range." % index)
+ def get_frames_at(self, *, indices):
+ import torch
+ frames = (
+ self._read_indexed(indices)
+ if self._keyframe_index is not None
+ else [self[index] for index in indices]
+ )
+ return SimpleNamespace(data=torch.stack(frames))
+
+ def _read_indexed(self, indices):
+ requested = [operator.index(index) for index in indices]
+ if any(index < 0 for index in requested):
+ raise IndexError("Video frame index is out of range.")
+ missing = sorted(set(requested) - set(self._cache))
+ decoded = self._decode_sparse(missing) if missing else {}
+ try:
+ return [
+ self._cache[index] if index in self._cache else decoded[index]
+ for index in requested
+ ]
+ except KeyError as error:
+ raise IndexError(
+ "Video frame index %d is out of range." % error.args[0]
+ ) from error
+
+ def _decode_sparse(self, indices):
+ decoded = {}
+ import av
+ pending = list(indices)
+ last_error = None
+ for retry in range(3):
+ groups, ranges = self._sparse_plan(pending, retry)
+ bodies = self._read_video_ranges(ranges)
+ failed = []
+ with tempfile.TemporaryFile() as sparse:
+ sparse.truncate(self._video_length())
+ for (offset, unused_length), body in zip(ranges, bodies):
+ sparse.seek(offset)
+ sparse.write(body)
+ sparse.seek(0)
+ with av.open(sparse) as container:
+ stream = container.streams.video[0]
+ for anchor, targets in sorted(groups.items()):
+ try:
+ self._decode_sparse_group(
+ container,
+ stream,
+ anchor,
+ sorted(targets),
+ decoded,
+ )
+ except (av.error.FFmpegError, ValueError) as error:
+ last_error = error
+ failed.extend(targets)
+ if not failed:
+ return decoded
+ pending = failed
+ raise ValueError(
+ "Cannot decode video frames from persisted seek index."
+ ) from last_error
Review Comment:
[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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
