[Other] support video_fps args for video bench (#7077)#7207
[Other] support video_fps args for video bench (#7077)#7207Wanglongzhi2001 wants to merge 1 commit intoPaddlePaddle:release/2.5from
Conversation
Wanglongzhi2001
had a problem deploying
to
Metax_ci
GitHub Actions
Failure
|
Thanks for your contribution! |
| collect_metrics: Optional[bool] = False | ||
|
|
||
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
video_fps 被加在 collect_metrics 之后、且不在 doc: start/end-*-extra-params 标记区域内。该文件多处用这些 doc 标记维护“额外参数”分组；把新增字段放在标记外会让参数分组/自动文档生成（如果有）变得不一致，后续也更难维护。建议把 video_fps 放入对应的 extra-params 区块，并补充一句英文注释说明单位/含义（fps 的语义是采样 fps 还是原视频 fps）。
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
新增 video_fps 字段会改变对外请求协议，但当前仓库里已有针对 CompletionRequest/ChatCompletionRequest 的单测（例如 tests/entrypoints/openai/test_chatcompletion_request.py）。建议补一条覆盖：1) 该字段能被正常解析；2) to_dict_for_infer() 会按预期透传该字段；3) （若加入范围约束）非法值能被拒绝。否则后续重构协议/序列化时容易回归。
| trace_context: Optional[str] = None | ||
|
|
||
| collect_metrics: Optional[bool] = False | ||
|
|
||
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
同上，建议把 video_fps 放到 doc: start/end-chat-completion-extra-params 区块内，以保持参数分组一致性；并补充英文说明字段语义/单位，避免与现有多模态 item 内部的 fps 概念混淆。
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
建议补充单测覆盖 ChatCompletionRequest.video_fps 的解析与透传行为（以及若添加约束后的错误路径）。目前该模块在 tests/entrypoints/openai 下已有较完整覆盖，新增协议字段不加测试容易产生回归。
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
从当前仓库代码来看，video_fps 仅在协议模型里新增了字段并会被 to_dict_for_infer() 透传，但在后续预处理/多模态处理链路中并没有任何地方读取 request['video_fps']（全仓搜索仅命中本文件与若干 data_processor 的初始化参数）。如果该参数期望影响视频抽帧，建议在构造/规范化 multimodal_data（或 mm item）时把 video_fps 映射为每个 video item 的 fps 默认值（例如当 item 未显式给 fps 时才填充），否则对外“支持 video_fps”可能只是接收字段但不生效。
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None | ||
|
|
There was a problem hiding this comment.
同上：ChatCompletionRequest.video_fps 当前只会被解析/透传，但仓库内没有消费该字段来影响视频抽帧参数（未映射到 mm item 的 fps 等）。若 benchmark 依赖该参数生效，建议在请求预处理阶段把 video_fps 下沉到具体 video item 的抽帧参数里（保持 item 级参数优先），否则会出现“参数可传但实际不生效”的问题。
fastdeploy-bot
left a comment
fastdeploy-bot
left a comment
There was a problem hiding this comment.
🤖 AI Code Review |
2026-04-07 15:18 CST
📋 Review 摘要
PR 概述：为视频理解 benchmark 添加 video_fps 参数支持
变更范围：entrypoints/openai/protocol.py
影响面 Tag：APIServer
📝 PR 规范检查
标题中的 Tag [Other] 不在官方 Tag 列表中，建议修改为 [Others]。
标题建议（可直接复制）：
[Others] support video_fps args for video bench (#7077)
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🟡 建议 | protocol.py:557 |
video_fps 类型使用 float，但下游处理器均使用 int |
总体评价
变更简单清晰，为 CompletionRequest 和 ChatCompletionRequest 添加了临时的 video_fps 参数用于视频理解 benchmark。由于 to_dict_for_infer 使用 self.dict() 自动遍历字段，新参数会被正确传递。建议确认类型是否需要与下游处理器保持一致。
| collect_metrics: Optional[bool] = False | ||
|
|
||
| # NOTE(Wanglongzhi2001): temporary parameter for video understanding benchmark | ||
| video_fps: Optional[float] = None |
There was a problem hiding this comment.
🟡 建议 类型一致性问题：此处 video_fps 使用 float 类型，但下游处理器中均使用 int 类型。
例如：
ernie4_5_vl_processor/process.py:105:video_fps: int = 2qwen_vl_processor/process.py:69:video_fps: int = FPSqwen3_vl_processor/process.py:147:video_fps: int = FPS
虽然 Python 会自动转换，但建议保持类型一致以避免潜在问题：
video_fps: Optional[int] = None
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/2.5 #7207 +/- ##
==============================================
Coverage ? 69.20%
==============================================
Files ? 390
Lines ? 54362
Branches ? 8570
==============================================
Hits ? 37619
Misses ? 14033
Partials ? 2710
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Motivation
Support video_fps args for video bench
Modifications
Support video_fps args for video bench
Usage or Command
Accuracy Tests
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.