Refactor target_fps handling in TorchrunInferenceWorker

[GACLove]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @GACLove, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the handling of the target_fps parameter within the TorchrunInferenceWorker to improve robustness and clarity. The changes ensure that target_fps is processed more safely and integrated into the video_frame_interpolation configuration without unintended side effects, while also providing explicit warnings when target_fps is set without the necessary video frame interpolation configuration.

Highlights

• Robust target_fps handling: The target_fps parameter is now extracted using task_data.pop(), ensuring it's removed from the task data after processing and preventing potential re-use or side effects.
• Improved video_frame_interpolation configuration: The target_fps is now merged into a new dictionary copy of the video_frame_interpolation configuration using dictionary unpacking, preventing direct modification of the runner's original configuration object.
• Enhanced error feedback: A warning is now logged if target_fps is provided in the task data but the video_frame_interpolation configuration is not present in the runner's settings, providing clearer feedback for misconfigurations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the handling of target_fps in TorchrunInferenceWorker. The changes make the code more concise by using pop and improve diagnostics by adding a warning when target_fps is provided without a video_frame_interpolation configuration. However, I've identified a potential bug in the new logic. The check for the existence of video_frame_interpolation configuration can fail if it's an empty dictionary, which would be a regression from the previous behavior. My review includes a suggestion to fix this.

[gemini-code-assist]

The check if vfi_cfg: will evaluate to False if video_frame_interpolation is configured as an empty dictionary ({}). In this scenario, the code will incorrectly log a warning and fail to set the target_fps, which is likely not the intended behavior. The previous implementation handled this case correctly by checking for key existence. To fix this and align with the probable intent, you should check if vfi_cfg is not None instead of checking for its truthiness.

Suggested change

	if vfi_cfg:
	task_data["video_frame_interpolation"] = {**vfi_cfg, "target_fps": target_fps}
	else:
	logger.warning(f"Target FPS {target_fps} is set, but video frame interpolation is not configured")
	if vfi_cfg is not None:
	task_data["video_frame_interpolation"] = {**vfi_cfg, "target_fps": target_fps}
	else:
	logger.warning(f"Target FPS {target_fps} is set, but video frame interpolation is not configured")