Skip to content

feat: Enhanced compatibility and performance in ShotRS2VPipeline#970

Merged
helloyongyang merged 1 commit intomainfrom
gp/fix
Mar 31, 2026
Merged

feat: Enhanced compatibility and performance in ShotRS2VPipeline#970
helloyongyang merged 1 commit intomainfrom
gp/fix

Conversation

@GACLove
Copy link
Copy Markdown
Contributor

@GACLove GACLove commented Mar 31, 2026

by moving generated video/audio segments to CPU and converting to float format before appending.

…oving generated video/audio segments to CPU and converting to float format before appending.
@GACLove GACLove requested review from helloyongyang, huochaitiantang and wangshankun and removed request for huochaitiantang March 31, 2026 11:36
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates rs2v_infer.py to move video and audio segments to the CPU and cast video segments to float before appending them to result lists. The review feedback points out a redundant .clone() call on the video segment and recommends adding a .float() conversion to the audio segment for consistency.

Comment on lines +158 to +159
gen_video_list.append(video_seg.clone().cpu().float())
cut_audio_list.append(audio_seg.cpu())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The implementation can be improved for efficiency and consistency:

  1. Redundant .clone(): On line 158, video_seg.clone() is unnecessary. Moving a tensor to CPU via .cpu() already creates a new copy. Since video_seg is a slice, .cpu() will only transfer the relevant data, making the GPU-side clone redundant.
  2. Consistency with PR description: The PR description mentions converting segments to float format before appending. This is missing for the audio segment on line 159. Adding .float() ensures consistency and aligns with the PR's objective.

Note: Applying these changes will make the subsequent conversions at lines 168 and 170 redundant, which you may want to clean up as well.

Suggested change
gen_video_list.append(video_seg.clone().cpu().float())
cut_audio_list.append(audio_seg.cpu())
gen_video_list.append(video_seg.cpu().float())
cut_audio_list.append(audio_seg.cpu().float())

@helloyongyang helloyongyang merged commit 5eccdeb into main Mar 31, 2026
2 checks passed
@helloyongyang helloyongyang deleted the gp/fix branch March 31, 2026 11:45
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