JingsongLi commented on code in PR #9831:
URL: https://github.com/apache/paimon/pull/9831#discussion_r4183112367
##########
paimon-python/pypaimon/multimodal/lerobot/dataset.py:
##########
@@ -1598,6 +1812,142 @@ 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_indexed(missing) if missing else {}
Review Comment:
[P2] Keep requested cache hits alive while decoding a batch
missing excludes frames already in the eight-entry LRU, but _decode_indexed
calls _remember for newly decoded frames and can evict those cache hits before
this method assembles its result. They are then absent from both self._cache
and decoded, so valid frame requests raise IndexError. I reproduced this with a
real 32-frame H.264 MP4 (B-frames, GOP 8), a persisted index generated by
VideoKeyframeIndex.inspect, and PyAV/Torch: decoder[0] followed by
get_frames_at(indices=range(16)) reports frame 0 out of range on both explicit
indexed PyAV and the default backend. A fresh indexed decoder reading the same
batch, and an unindexed decoder with the same warm-up, both return all 16
tensors identical to complete PyAV decoding. Overlapping training windows or
mixed cached/random batches can hit this normally. Preserve a request-local
snapshot of requested cached frames before decoding, and combine it with
decoded results; add a warm-cache batch larger than the LRU regression test.
##########
paimon-python/pypaimon/table/row/blob.py:
##########
@@ -229,18 +229,27 @@ class VideoFrameDescriptor(BlobDescriptor):
CURRENT_VERSION = 1
MAGIC = 0x564944454F46524D # "VIDEOFRM"
- _FIXED_LENGTH = 1 + 8 + 4 + 8 + 8 + 8
+ _FIXED_LENGTH = 1 + 8 + 4 + 8 + 8 + 8 + 8 + 8
- def __init__(self, uri: str, offset: int, length: int, frame_index: int):
+ def __init__(
+ self, uri: str, offset: int, length: int, frame_index: int,
+ keyframe_index_offset: int, keyframe_index_length: int):
Review Comment:
[P2] Update the remaining no-index descriptor consumers
The two new mandatory arguments break the existing four-argument
VideoFrameDescriptor construction. ray_blob_affinity_test.py still uses that
public exported constructor in
test_video_frames_share_payload_uri_key_and_preserve_offsets and
test_video_prefetch_windows_use_payload_length; both now fail with TypeError
before testing the Ray/video integration. I ran the exact two cases: current
head fails both, while substituting the base blob.py passes both. The current
Python 3.10-3.13 and Native CI failures have this same cause. The scan fallback
still supports videos without an index, so either supply safe no-index defaults
(-1, 0) for existing callers, or deliberately migrate the remaining consumers
and document the constructor change; ensure these integration checks pass
alongside the new format tests.
--
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]
