merge#32
Conversation
fix the new issue for the codec of webm and mp4 and m4a
fix the playlist issue and related issues
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
Summary of ChangesHello, 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 focuses on improving the user experience and maintainability of the Predator application. It includes a comprehensive update to the README, detailing features and usage, enhancements to video/audio format selection logic, and the addition of a contributing guide. Highlights
Ignored Files
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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request appears to be a merge of a feature branch, which includes a major refactoring from the Fyne framework to Wails. The README.md has been significantly improved and a CONTRIBUTING.md file has been added. The download logic in app.go has been made more robust with better format selection fallbacks.
My review includes a few suggestions:
- In
README.md, there are some inconsistencies regarding the required Go version and a broken link in the table of contents. - In
app.go, I've identified an unused function parameter, some duplicated code that can be refactored, and a new function that seems to be dead code.
Overall, these are great improvements to the project's structure and functionality. Addressing these points will enhance code quality and documentation clarity.
| func buildWebmFormatString(cleanRes string) string { | ||
| // For webm, we want VP9 or AV1 video with opus audio | ||
| // This provides better quality than older vp8/opus combinations | ||
|
|
||
| if cleanRes == "best" { | ||
| // Best quality webm - prioritize av01 (AV1) then vp9 | ||
| return "bestvideo[vcodec^=av01]+bestaudio[acodec^=opus]/" + | ||
| "bestvideo[vcodec^=av01]+bestaudio[ext=webm]/" + | ||
| "bestvideo[vcodec^=av01]+bestaudio/" + | ||
| "bestvideo[vcodec^=vp9]+bestaudio[acodec^=opus]/" + | ||
| "bestvideo[vcodec^=vp9]+bestaudio[ext=webm]/" + | ||
| "bestvideo[vcodec^=vp9]+bestaudio/" + | ||
| "bestvideo[vcodec^=vp8]+bestaudio[acodec^=opus]/" + | ||
| "bestvideo[vcodec^=vp8]+bestaudio/" + | ||
| "bestvideo+bestaudio[acodec^=opus]/bestvideo+bestaudio/best" | ||
| } | ||
|
|
||
| resNum, _ := strconv.Atoi(cleanRes) | ||
| if resNum >= 1440 { | ||
| // High resolutions - prioritize av01 then vp9 | ||
| return fmt.Sprintf( | ||
| // AV1 with opus - best quality | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio/"+ | ||
| // AV1 <= height fallback | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio/"+ | ||
| // VP9 with opus | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio/"+ | ||
| // VP9 <= height fallback | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio/"+ | ||
| // Any codec fallback | ||
| "bestvideo[height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[height<=%s]+bestaudio/"+ | ||
| "best[height<=%s]", | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, | ||
| ) | ||
| } | ||
|
|
||
| // Standard resolutions - prioritize av01 then vp9 with opus | ||
| return fmt.Sprintf( | ||
| // AV1 with opus | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio/"+ | ||
| // AV1 <= height fallback | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio/"+ | ||
| // VP9 with opus | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio/"+ | ||
| // VP9 <= height fallback | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio/"+ | ||
| // Any codec fallback | ||
| "bestvideo[height<=%s]+bestaudio[acodec^=opus]/"+ | ||
| "bestvideo[height<=%s]+bestaudio[ext=webm]/"+ | ||
| "bestvideo[height<=%s]+bestaudio/"+ | ||
| "best[height<=%s]", | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, | ||
| ) | ||
| } |
| - [Features](#features) | ||
| - [Getting Started](#getting-started) | ||
| - [How to Use](#how-to-use) | ||
| - [Keyboard Shortcuts](#keyboard-shortcuts) |
| ## Getting Started | ||
|
|
||
| ## Installation | ||
| You'll need **Go 1.24 or newer** installed. |
There was a problem hiding this comment.
|
|
||
| // buildVideoFormatString builds format string for mp4 container with proper codec handling | ||
| // Uses separate video+audio selection to ensure reliable merging | ||
| func buildVideoFormatString(cleanRes string, preferH264 bool) string { |
| resNum, _ := strconv.Atoi(cleanRes) | ||
| if resNum >= 1440 { | ||
| // High resolutions - try exact height first, then fall back to <= | ||
| // STRICTLY m4a audio only to prevent ffmpeg merge failures | ||
| // High resolutions (1440p+) - prioritize h264 with exact or close height match | ||
| return fmt.Sprintf( | ||
| // Try exact height match first with m4a audio only | ||
| // h264 with m4a audio - best for mp4 merging | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio/"+ | ||
| // h264 with <= height fallback | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio/"+ | ||
| // vp9 fallback | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio/"+ | ||
| // av1 fallback | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| // Fall back to <= if exact match not available - still m4a only | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| // Final fallbacks - still prefer m4a | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio/"+ | ||
| // Any video codec fallback | ||
| "bestvideo[height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[height<=%s]+bestaudio/"+ | ||
| "best[height<=%s]", | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, | ||
| ) | ||
| } | ||
|
|
||
| // Standard resolutions - try exact height first, then fall back | ||
| // STRICTLY m4a audio only to prevent ffmpeg merge failures | ||
| // Standard resolutions - prioritize h264 with m4a audio | ||
| return fmt.Sprintf( | ||
| // Try exact height match first with m4a audio only | ||
| // h264 with m4a audio - best for mp4 merging | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height=%s]+bestaudio/"+ | ||
| // h264 with <= height fallback | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio/"+ | ||
| // vp9 fallback | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height=%s]+bestaudio/"+ | ||
| // av1 fallback | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| // Fall back to <= if exact match not available - still m4a only | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=avc1][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=vp9][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[vcodec^=av01][height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| // Final fallbacks - still prefer m4a | ||
| "bestvideo[vcodec^=av01][height=%s]+bestaudio/"+ | ||
| // Any video codec fallback | ||
| "bestvideo[height<=%s]+bestaudio[ext=m4a]/"+ | ||
| "bestvideo[height<=%s]+bestaudio[acodec^=mp4a]/"+ | ||
| "bestvideo[height<=%s]+bestaudio/"+ | ||
| "best[height<=%s]", | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, | ||
| cleanRes, cleanRes, cleanRes, cleanRes, | ||
| ) |
There was a problem hiding this comment.
The code blocks for high resolutions (>= 1440p) and standard resolutions are identical. This duplication can be removed by merging them into a single block, which will make the code more maintainable.
// The logic for all resolutions is the same, so no need for the if/else block.
// Standard resolutions - prioritize h264 with m4a audio
return fmt.Sprintf(
// h264 with m4a audio - best for mp4 merging
"bestvideo[vcodec^=avc1][height=%s]+bestaudio[ext=m4a]/"+
"bestvideo[vcodec^=avc1][height=%s]+bestaudio[acodec^=mp4a]/"+
"bestvideo[vcodec^=avc1][height=%s]+bestaudio/"+
// h264 with <= height fallback
"bestvideo[vcodec^=avc1][height<=%s]+bestaudio[ext=m4a]/"+
"bestvideo[vcodec^=avc1][height<=%s]+bestaudio[acodec^=mp4a]/"+
"bestvideo[vcodec^=avc1][height<=%s]+bestaudio/"+
// vp9 fallback
"bestvideo[vcodec^=vp9][height=%s]+bestaudio[ext=m4a]/"+
"bestvideo[vcodec^=vp9][height=%s]+bestaudio[acodec^=mp4a]/"+
"bestvideo[vcodec^=vp9][height=%s]+bestaudio/"+
// av1 fallback
"bestvideo[vcodec^=av01][height=%s]+bestaudio[ext=m4a]/"+
"bestvideo[vcodec^=av01][height=%s]+bestaudio[acodec^=mp4a]/"+
"bestvideo[vcodec^=av01][height=%s]+bestaudio/"+
// Any video codec fallback
"bestvideo[height<=%s]+bestaudio[ext=m4a]/"+
"bestvideo[height<=%s]+bestaudio[acodec^=mp4a]/"+
"bestvideo[height<=%s]+bestaudio/"+
"best[height<=%s]",
cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes,
cleanRes, cleanRes, cleanRes, cleanRes, cleanRes, cleanRes,
cleanRes, cleanRes, cleanRes, cleanRes,
)
No description provided.