Skip to content

feat(dashscope): support video fps option for chat model - #293

Open
charliejinc wants to merge 2 commits into
spring-ai-alibaba:mainfrom
charliejinc:fix/dashscope-chat-video-fps
Open

feat(dashscope): support video fps option for chat model#293
charliejinc wants to merge 2 commits into
spring-ai-alibaba:mainfrom
charliejinc:fix/dashscope-chat-video-fps

Conversation

@charliejinc

Copy link
Copy Markdown

Description

Add fps (frame extraction rate) support for video content in the DashScope multimodal chat model.

Changes

  • Added fps field to DashScopeChatOptions for default video frame-rate configuration.
  • Added VIDEO_FPS metadata key to DashScopeApiConstants for per-message override.
  • Added fps field to DashScopeApiSpec.MediaContent so it is serialized into video content items.
  • DashScopeChatModel now passes fps when building video content items; message metadata VIDEO_FPS takes precedence over DashScopeChatOptions.getFps().
  • Added integration tests for single-video fps and message-level metadata override.
  • Fixed DashScopeApiSpecTests class javadoc.

Usage

// Option 1: via default options
DashScopeChatOptions options = DashScopeChatOptions.builder()
        .model("qwen-vl-max")
        .multiModel(true)
        .fps(2)
        .build();

// Option 2: override per message via metadata
UserMessage userMessage = UserMessage.builder()
        .text("Describe this video")
        .media(videoMedia)
        .build();
userMessage.getMetadata().put(DashScopeApiConstants.VIDEO_FPS, 1);

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6b8c9db16

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +653 to +654
if (metadataFps instanceof Number num) {
return num.intValue();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve fractional video fps values

When callers set VIDEO_FPS to a valid fractional value such as 0.5 for a long/static video, this branch coerces any Number with intValue(), so 0.5 becomes 0, and the string path rejects "0.5" with Integer.parseInt. DashScope documents fps as a float in the range [0.1, 10] with default 2.0 (https://help.aliyun.com/zh/model-studio/qwen-api-via-dashscope), so these requests either fail validation or use the wrong sampling rate; store and serialize fps as a floating-point number instead.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

Add fps (frame extraction rate) support for video content in the DashScope multimodal chat model.

Changes:

- Add fps field to DashScopeChatOptions for default video frame-rate configuration.

- Add VIDEO_FPS metadata key to DashScopeApiConstants for per-message override.

- Add fps field to DashScopeApiSpec.MediaContent so it is serialized into video content items.

- Pass fps when building video content items; message metadata VIDEO_FPS takes precedence over DashScopeChatOptions.getFps().

- Add integration tests for single-video fps and message-level metadata override.

- Fix DashScopeApiSpecTests class javadoc.
@charliejinc
charliejinc force-pushed the fix/dashscope-chat-video-fps branch from c6b8c9d to d2558ae Compare July 20, 2026 09:50
@charliejinc

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@charliejinc

charliejinc commented Jul 20, 2026 via email

Copy link
Copy Markdown
Author

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2558ae6be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +995 to 996
@JsonProperty("audio") String audio, @JsonProperty("fps") Double fps,
@JsonProperty("cache_control") Map<String, String> cacheControl) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the existing MediaContent constructor

When downstream code uses the public record constructor that existed before this commit (new MediaContent(type, text, image, video, audio, cacheControl)), inserting fps before cacheControl removes the old (String, String, String, Object, String, Map) signature. Upgrading this library can therefore break source compatibility, or make already-compiled callers fail with NoSuchMethodError, even if they do not use fps; please keep an overload with the old signature that delegates with a null fps.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Useful ,Fixed

…ility

Keep the (type, text, image, video, audio, cacheControl) overload so
downstream callers compiled against the old signature don't break.
yuluo-yx added a commit to spring-ai-alibaba/cla that referenced this pull request Jul 21, 2026

@charliejinc charliejinc left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

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.

1 participant