Skip to content

fix(oryx): make the video call sites match what load_video/read_video return - #1493

Merged
pufanyi merged 1 commit into
EvolvingLMMs-Lab:mainfrom
Anai-Guo:fix/oryx-video-load-return-contract
Aug 31, 2026
Merged

fix(oryx): make the video call sites match what load_video/read_video return#1493
pufanyi merged 1 commit into
EvolvingLMMs-Lab:mainfrom
Anai-Guo:fix/oryx-video-load-return-contract

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Problem

Oryx.load_video returns a 2-tuple:

# lmms_eval/models/simple/oryx.py:207
def load_video(self, video_path, max_frames_num):
    ...
    return spare_frames, modality  # (frames, height, width, channels)

and the shared helper read_video returns a bare np.ndarray:

# lmms_eval/models/model_utils/load_video.py
def read_video(video_path, *, num_frm=8, ...) -> np.ndarray:

Two call sites in oryx.py disagree with those contracts, in two different ways.

1. generate_until, the pyav branch (line 355)

if self.video_decode_backend == "decord":
    video, modality = self.load_video(visual, self.max_frames_num)   # correct: 2-tuple
elif self.video_decode_backend == "pyav":
    video, modality = read_video(visual, num_frm=self.max_frames_num)  # read_video returns one array

read_video hands back (N, H, W, 3), so the unpack splits it along the frame axis:

  • with the default max_frames_num=32 it raises ValueError: too many values to unpack (expected 2);
  • for a video sampled down to exactly 2 frames it does not raise — video silently
    becomes a single (H, W, 3) frame and modality becomes another raw frame instead of
    the string "video". The following for frame in video: then iterates over pixel rows,
    and modalities.append(modality) appends an array where the model expects a tag.

The decord branch three lines above is the correct sibling; the pyav branch was
evidently copied from it without adapting to read_video's return type.

2. loglikelihood (line 238)

video = self.load_video(visual, self.max_frames_num)
video = self._image_processor.preprocess(video, return_tensors="pt")["pixel_values"]...

This drops the unpack entirely, so the image processor is handed the (ndarray, str)
tuple rather than the frames. The same method is unpacked correctly at line 353.

Fix

Two call sites, matching what each callee actually returns:

  • line 355 — take the frames from read_video and set modality = "video" explicitly,
    which is exactly what load_video (line 214) and the image-list branch (line 342) do.
  • line 238 — unpack the pair and discard the modality, which loglikelihood does not use.

No behavior change on the decord path.

Verification

Both failures are return-contract mismatches, so no GPU or model weights are needed.
load_video's return arity was read out of the file's own AST and read_video's from its
signature, then the unpacks were replayed:

Oryx.load_video returns: return (spare_frames, modality) -> tuple of 2
read_video returns     : <class 'numpy.ndarray'>

--- oryx.py:355  `video, modality = read_video(visual, num_frm=32)` ---
  ValueError: too many values to unpack (expected 2)
  num_frm=2 -> modality is a ndarray(4, 4, 3), not the string 'video'

--- oryx.py:238  `video = self.load_video(...)` then preprocess(video) ---
  video is tuple of ['ndarray', 'str'] -> the image processor is handed a (ndarray, str) tuple

--- CONTROL: oryx.py:353 decord sibling `video, modality = self.load_video(...)` ---
  unpack OK -> video ndarray(32, 4, 4, 3), modality='video'

--- FIXED forms ---
  read_video(visual, num_frm=...) binds OK; assign to `video` alone
  video, _modality = self.load_video(...) -> video ndarray(32, 4, 4, 3)

ruff format and ruff check at the version pinned in .pre-commit-config.yaml (v0.16.4),
against the repo's pyproject.toml: format clean, and the 17 pre-existing findings in this
file are byte-for-byte unchanged by the patch.

Related: #1492 fixes the same copied-from-decord mistake in llava_vid.py's pyav branch.


🤖 Generated with Claude Code

… return

load_video returns (frames, modality) and read_video returns only the
frame array; two call sites disagreed with both.
@pufanyi
pufanyi self-requested a review August 31, 2026 07:34
@pufanyi

pufanyi commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Thanks for your contribution! I will merge it.

@pufanyi
pufanyi merged commit 6079aa6 into EvolvingLMMs-Lab:main Aug 31, 2026
2 checks passed
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