Conversation
Summary of ChangesHello @mose-zm, 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 video synthesis capabilities by integrating audio input. It introduces a dedicated 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
|
There was a problem hiding this comment.
Code Review
This pull request adds support for an audio_url parameter to the video synthesis API, allowing users to provide an audio track for video generation. The changes are applied consistently across both synchronous and asynchronous methods, and a new sample script is added to demonstrate the feature. My review focuses on improving documentation, code maintainability, and adherence to Python best practices in the new sample code.
| negative_prompt (str): The negative prompt is the opposite of the prompt meaning. | ||
| template (str): LoRa input, such as gufeng, katong, etc. | ||
| img_url (str): The input image url, Generate the URL of the image referenced by the video. | ||
| audio_url (str): The input audio url |
There was a problem hiding this comment.
The docstring for audio_url is a bit brief. For better clarity and consistency with other parameters like img_url, consider providing a more descriptive explanation of its purpose. Also, this docstring is missing a period at the end. This feedback also applies to the audio_url docstrings on lines 209, 378, and 432.
| audio_url (str): The input audio url | |
| audio_url (str): The input audio URL, used for video generation. |
| if audio_url is not None and audio_url: | ||
| is_upload, res_audio_url = check_and_upload_local( | ||
| model, audio_url, api_key) | ||
| if is_upload: | ||
| has_upload = True | ||
| inputs['audio_url'] = res_audio_url |
There was a problem hiding this comment.
This logic for handling audio_url is very similar to the logic for other URL-based parameters like img_url, head_frame, etc., leading to code duplication. This can make the code harder to maintain and prone to errors if changes are not applied consistently across all blocks.
To improve maintainability, consider refactoring this repeated logic into a private helper method. For example:
@classmethod
def _maybe_upload_and_set_input(cls, inputs, key, url, model, api_key):
if not url:
return False
is_upload, res_url = check_and_upload_local(model, url, api_key)
inputs[key] = res_url
return is_uploadThen you could use it like this:
if cls._maybe_upload_and_set_input(inputs, 'audio_url', audio_url, model, api_key):
has_upload = TrueThis would make the _get_input method much cleaner and easier to read. Also, the check if audio_url is not None and audio_url: can be simplified to if audio_url:.
| audio_url=audio_url) | ||
| if rsp.status_code == HTTPStatus.OK: | ||
|
|
||
| print('response: %s' % rsp) |
| print('sync_call Failed, status_code: %s, code: %s, message: %s' % | ||
| (rsp.status_code, rsp.code, rsp.message)) |
There was a problem hiding this comment.
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| simple_call() No newline at end of file |
Add input.audio_url to support wan2.5 video generation