[feat] Add gradio local inference demo #847
Conversation
Summary of ChangesHello @SolitaryThinker, I'm Gemini Code Assist1! 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 significantly enhances the FastVideo framework by integrating a new Gradio-based local inference demo. This demo provides a user-friendly web interface for generating videos from text prompts, complete with various customization options and performance metrics. Additionally, the changes include improvements to video output file handling and a clearer structure for Gradio examples within the repository. Highlights
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Gradio-based local inference demo, which is a great addition for showcasing the capabilities of FastVideo. The implementation is comprehensive, including a web UI, backend logic, and supporting assets. My review focuses on improving robustness, portability, and maintainability. Key suggestions include fixing a critical import error, addressing hardcoded paths, improving the video-saving logic to be more reliable, and making the code less brittle. There are also some suggestions to enhance the API of VideoGenerator to make it more user-friendly.
| params.width = int(width) | ||
|
|
||
| if randomize_seed: | ||
| params.seed = torch.randint(0, 1000000, (1, )).item() |
There was a problem hiding this comment.
The torch library is used here to generate a random integer, but it has not been imported in this file. This will raise a NameError when randomize_seed is checked. You should add import torch at the top of the file to resolve this.
| print(f"Warning: Could not read {filepath}: {e}") | ||
| return prompts, labels | ||
|
|
||
| examples, example_labels = load_from_file("/FastVideo/examples/inference/gradio/local/prompts_final.txt") |
There was a problem hiding this comment.
Using a hardcoded absolute path /FastVideo/examples/inference/gradio/local/prompts_final.txt will cause the script to fail on any machine where the project is not located at the root of the filesystem. The path should be relative to the script's location to ensure portability.
| examples, example_labels = load_from_file("/FastVideo/examples/inference/gradio/local/prompts_final.txt") | |
| script_dir = os.path.dirname(os.path.realpath(__file__)) | |
| examples, example_labels = load_from_file(os.path.join(script_dir, "prompts_final.txt")) |
| safe_prompt = params.prompt[:100].replace(' ', '_').replace('/', '_').replace('\\', '_') | ||
| video_filename = f"{params.prompt[:100]}.mp4" | ||
| output_path = os.path.join(output_dir, video_filename) |
There was a problem hiding this comment.
There are a couple of issues with how the output video path is handled here:
- The
safe_promptvariable is created but never used. This is dead code. - The
video_filenameis constructed from the raw prompt, which may contain characters that are invalid in filenames (e.g.,/,?,*). This could cause an error when Gradio tries to serve the file. - This logic does not account for filename collisions. The
VideoGeneratorhandles this by appending suffixes like_1,_2, etc., to the filename if it already exists. Because this script doesn't replicate that logic, it might return a path to an older video if a new video was saved with a different name due to a collision.
A more robust solution would be to have VideoGenerator.generate_video return the final path of the saved video and use that path here. I've added a separate comment in fastvideo/entrypoints/video_generator.py with a suggestion to implement this.
| } | ||
|
|
||
| def create_timing_display(inference_time, total_time, stage_execution_times, num_frames): | ||
| dit_denoising_time = f"{stage_execution_times[5]:.2f}s" if len(stage_execution_times) > 5 else "N/A" |
There was a problem hiding this comment.
Accessing stage_execution_times[5] with a hardcoded index is brittle. If the order or number of pipeline stages changes in the future, this will either fetch the wrong timing information or cause an IndexError. It would be more robust to find the index of the 'DiT Denoising' stage dynamically from the stage_names list, which is already being retrieved from logging_info.
| app, | ||
| demo, | ||
| path="/gradio", | ||
| allowed_paths=[os.path.abspath("outputs"), os.path.abspath("fastvideo-logos")] |
There was a problem hiding this comment.
The path fastvideo-logos in allowed_paths seems incorrect. The logo asset assets/full.svg is used in the Gradio interface, which is located in the assets directory. To allow Gradio to serve this file, you should include the assets directory in allowed_paths.
| allowed_paths=[os.path.abspath("outputs"), os.path.abspath("fastvideo-logos")] | |
| allowed_paths=[os.path.abspath("outputs"), os.path.abspath("assets")] |
| print( | ||
| f"The video name '{os.path.basename(output_path)}' contained invalid characters. It has been renamed to '{video_name}'" | ||
| ) |
There was a problem hiding this comment.
Using print() for logging is inconsistent with the rest of the codebase, which uses the logging module. It's better to use logger.info() or logger.warning() for maintainability and to allow for centralized log level control.
| print( | |
| f"The video name '{os.path.basename(output_path)}' contained invalid characters. It has been renamed to '{video_name}'" | |
| ) | |
| logger.info( | |
| "The video name '%s' contained invalid characters. It has been renamed to '%s'", | |
| os.path.basename(output_path), video_name | |
| ) |
2 participants
replaces #740