fix: a failed subtitle no longer destroys the download
Two faults behind the 429 in the report. The subtitle languages were a wildcard: 'en.*' also matches every machine-translated variant YouTube offers — en-en-US and dozens more — so each download fired a burst of subtitle requests and earned an HTTP 429. Languages are now named exactly, as '<lang>,<lang>-orig'. Turning subtitles off now really does request none; it previously still asked for English. Worse, yt-dlp exits non-zero if anything at all failed, and a caption it could not fetch was enough to mark a fully downloaded video as failed and leave the file orphaned. yt-dlp now runs with --ignore-errors, and success is judged by whether the media file actually landed rather than by the exit code. 429 also gets its own message pointing at the sign-in setting, which raises the limit.
This commit is contained in:
@@ -121,6 +121,11 @@ fn explain_yt_dlp_error(stderr: &str, signed_in: bool) -> String {
|
||||
.into()
|
||||
};
|
||||
}
|
||||
if stderr.contains("429") || stderr.contains("Too Many Requests") {
|
||||
return "YouTube is rate-limiting this machine. Wait a few minutes, or sign in \
|
||||
under Settings → Sign in to YouTube, which raises the limit."
|
||||
.into();
|
||||
}
|
||||
if stderr.contains("Operation not permitted") && stderr.contains("Safari") {
|
||||
return "macOS blocked access to Safari's cookies. Give FlightTube Full Disk Access in System Settings → Privacy & Security, or pick a different browser."
|
||||
.into();
|
||||
@@ -1038,15 +1043,15 @@ pub async fn download_video(
|
||||
let stderr_lines = stderr_task.await.unwrap_or_default();
|
||||
drop(permit);
|
||||
|
||||
if status.success() {
|
||||
// yt-dlp normally reports the path via `--print after_move:`; if that
|
||||
// line went missing, find the file it wrote by its embedded video id.
|
||||
let path = match final_path {
|
||||
Some(p) => p,
|
||||
None => find_by_video_id(&library, &video_id)
|
||||
.await
|
||||
.ok_or("Download finished but the file could not be located.")?,
|
||||
};
|
||||
// yt-dlp exits non-zero if *anything* failed, including a subtitle it could
|
||||
// not fetch. If the video itself landed, the download succeeded — throwing
|
||||
// away a finished file over a missing caption would be absurd.
|
||||
let landed = match final_path {
|
||||
Some(p) if tokio::fs::metadata(&p).await.is_ok() => Some(p),
|
||||
_ => find_by_video_id(&library, &video_id).await,
|
||||
};
|
||||
|
||||
if let Some(path) = landed {
|
||||
// YouTube's captions arrive pinned to the left edge and full of
|
||||
// karaoke timing tags; clean them before they reach the player.
|
||||
tidy_subtitles(&library, &video_id).await;
|
||||
|
||||
+71
-17
@@ -79,13 +79,15 @@ pub fn parse_progress_line(line: &str) -> Option<Progress> {
|
||||
|
||||
/// Arguments for downloading one video. Kept separate from process spawning so
|
||||
/// the argument construction is assertable in tests.
|
||||
/// `sub_langs` is empty when subtitles are switched off, in which case none
|
||||
/// are requested at all.
|
||||
pub fn build_args(
|
||||
video_id: &str,
|
||||
out_template: &str,
|
||||
quality: &str,
|
||||
sub_langs: &str,
|
||||
) -> Vec<String> {
|
||||
vec![
|
||||
let mut args = vec![
|
||||
"-f".into(),
|
||||
format_selector(quality),
|
||||
"--merge-output-format".into(),
|
||||
@@ -99,24 +101,50 @@ pub fn build_args(
|
||||
"180".into(),
|
||||
"--progress-template".into(),
|
||||
PROGRESS_TEMPLATE.into(),
|
||||
// Subtitles come along for offline use, including YouTube's
|
||||
// auto-generated ones. They are written beside the video as WebVTT
|
||||
// rather than muxed in: WebKit reads a <track> reliably, whereas
|
||||
// subtitle streams inside an MP4 it largely ignores.
|
||||
"--write-subs".into(),
|
||||
"--write-auto-subs".into(),
|
||||
"--sub-format".into(),
|
||||
"vtt".into(),
|
||||
"--convert-subs".into(),
|
||||
"vtt".into(),
|
||||
"--sub-langs".into(),
|
||||
sub_langs.into(),
|
||||
// A failing subtitle must never take the video with it. Subtitles are
|
||||
// a bonus; the file is the point.
|
||||
"--ignore-errors".into(),
|
||||
"--print".into(),
|
||||
"after_move:FTPATH %(filepath)s".into(),
|
||||
"-o".into(),
|
||||
out_template.into(),
|
||||
format!("https://www.youtube.com/watch?v={video_id}"),
|
||||
]
|
||||
];
|
||||
|
||||
if !sub_langs.is_empty() {
|
||||
// Subtitles come along for offline use, including YouTube's
|
||||
// auto-generated ones. WebVTT beside the video rather than muxed in:
|
||||
// WebKit reads a <track> reliably and largely ignores subtitle streams
|
||||
// inside an MP4.
|
||||
//
|
||||
// The languages are named exactly. A wildcard like "en.*" also matches
|
||||
// every machine-translated variant YouTube offers — en-en-US, en-de and
|
||||
// dozens more — and asking for all of them earns an HTTP 429.
|
||||
args.extend([
|
||||
"--write-subs".into(),
|
||||
"--write-auto-subs".into(),
|
||||
"--sub-format".into(),
|
||||
"vtt".into(),
|
||||
"--convert-subs".into(),
|
||||
"vtt".into(),
|
||||
"--sub-langs".into(),
|
||||
sub_langs.to_string(),
|
||||
]);
|
||||
}
|
||||
|
||||
args.push(format!("https://www.youtube.com/watch?v={video_id}"));
|
||||
args
|
||||
}
|
||||
|
||||
/// The subtitle languages to request for a preference, or empty for none.
|
||||
///
|
||||
/// Only the language itself and YouTube's "-orig" variant; anything broader
|
||||
/// pulls in machine translations by the dozen.
|
||||
pub fn sub_langs_for(pref: &str) -> String {
|
||||
if pref.is_empty() || pref == "off" {
|
||||
String::new()
|
||||
} else {
|
||||
format!("{pref},{pref}-orig")
|
||||
}
|
||||
}
|
||||
|
||||
/// yt-dlp reports the final path via `--print after_move:`, which is more
|
||||
@@ -194,15 +222,41 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn subtitles_are_requested_including_auto_generated() {
|
||||
let args = build_args("abc", "/tmp/o.%(ext)s", "best", "en.*,nl.*");
|
||||
let args = build_args("abc", "/tmp/o.%(ext)s", "best", "en,en-orig");
|
||||
assert!(args.contains(&"--write-subs".to_string()));
|
||||
// The auto-generated track is the only one many videos have.
|
||||
assert!(args.contains(&"--write-auto-subs".to_string()));
|
||||
assert!(args.contains(&"en.*,nl.*".to_string()));
|
||||
assert!(args.contains(&"en,en-orig".to_string()));
|
||||
// WebVTT, because that is what a <track> element can load.
|
||||
assert!(args.contains(&"vtt".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_preference_means_no_subtitle_requests_at_all() {
|
||||
let args = build_args("abc", "/tmp/o.%(ext)s", "best", "");
|
||||
assert!(!args.contains(&"--write-subs".to_string()));
|
||||
assert!(!args.contains(&"--write-auto-subs".to_string()));
|
||||
assert!(!args.contains(&"--sub-langs".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_subtitle_failure_must_not_abort_the_video() {
|
||||
// yt-dlp aborts the whole job on the first error without this.
|
||||
assert!(build_args("abc", "/tmp/o.%(ext)s", "best", "en,en-orig")
|
||||
.contains(&"--ignore-errors".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn languages_are_named_exactly_never_as_a_wildcard() {
|
||||
// "en.*" also matches en-en-US and dozens of machine translations,
|
||||
// and requesting them all earns an HTTP 429.
|
||||
let langs = sub_langs_for("nl");
|
||||
assert_eq!(langs, "nl,nl-orig");
|
||||
assert!(!langs.contains('*'));
|
||||
assert_eq!(sub_langs_for("off"), "");
|
||||
assert_eq!(sub_langs_for(""), "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn best_quality_takes_the_highest_available() {
|
||||
let args = build_args("abc123", "/tmp/out.%(ext)s", "best", "en.*");
|
||||
|
||||
Reference in New Issue
Block a user