fix: align with yt-dlp's behavior again - #724
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 984671fbc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Removed audio encoding from sort fields and updated test expectations.
|
btw i deprioritized because the current situation isnt permanent, @jely2002 thank you for ur review in advance |
jely2002
left a comment
jely2002
left a comment
There was a problem hiding this comment.
Rest looks fine, we'll see how it goes. I do think tests are failing.
| args.push("--no-embed-chapters".into()); | ||
| // Ensure we don't get black screens or missing audio. | ||
| args.push("--force-keyframes-at-cuts".into()); | ||
| if output_settings.video.policy == TranscodePolicy::AllowReencode { |
There was a problem hiding this comment.
Not a fan of this change, without this option we get very bad cuts and broken keyframes. If a user chooses to download only part of a video, we can just assume its okay to add the option.
There was a problem hiding this comment.
in our colleagues' environment, without it yt-dlp just downloads a bit wider sections than the setting, but doesnt have them broken.
and it forces reencoding, which i dont think its intended by the users who dont want it to reencode
|
|
||
| pub fn build_format_args( | ||
| format_options: &FormatOptions, | ||
| output_settings: &OutputSettings, |
There was a problem hiding this comment.
unused variable
|
i wonder if specifying the codec string from info to |
2 participants
acodec_filters