Fix medium-severity defects across download, formats and tooling

- Format table: a missing acodec was treated as "has audio", skipping
  the +bestaudio merge and producing silent videos for extractors that
  omit the field.
- Progress bar: separate video/audio stream downloads each reported
  0-100%, making the bar jump backwards; per-phase scaling now maps the
  two streams onto 0-50/50-100.
- Custom commands: parse with shlex (quoted arguments with spaces were
  shredded by str.split), keep POSIX mode off on Windows so backslash
  paths survive, hide the console window like every other call site,
  close the stdout pipe, and support cancellation of a running command.
- Settings dialog: _("settings", "error_saving", ...) passed two
  positional args to the i18n helper, raising TypeError inside the
  except handler instead of showing the intended error dialog.
- ffmpeg on Windows: Path(os.getenv("LOCALAPPDATA")) crashed with
  TypeError when the variable is unset; fall back to the standard
  AppData/Local location.
- Version cache: cached path (str) was compared against a Path, so the
  cache never hit and every version query spawned a subprocess.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jaroslav Beneš
2026-07-25 01:35:23 +02:00
parent 288d30ad8b
commit 4cc48ae98f
6 changed files with 49 additions and 14 deletions
+10
View File
@@ -113,6 +113,8 @@ class DownloadThread(QThread):
self.subtitle_files: List[str] = [] # Track subtitle files that are created
self.initial_subtitle_files: Set[Path] = set() # Track initial subtitle files before download
self.download_files: Set[Path] = set() # Every destination path this download wrote to
self.expected_phases: int = 1 # 2 when video and audio download separately before merge
self._media_phase: int = 0 # Index of the media stream currently downloading
def cleanup_partial_files(self) -> None:
"""Delete partial files (.part/.ytdl and unmerged .fNNN. streams), but only
@@ -293,6 +295,7 @@ class DownloadThread(QThread):
logger.debug(f"Using progressive format with bundled audio: {clean_format_id}")
else:
cmd.extend(["-f", f"{clean_format_id}+bestaudio/best"])
self.expected_phases = 2 # separate video and audio downloads
logger.debug(f"Using video-only format merged with best audio: {clean_format_id}+bestaudio/best")
else:
# If no specific format ID, use resolution-based sorting (-S)
@@ -635,6 +638,8 @@ class DownloadThread(QThread):
filepath = dest_match.group(1).strip()
self.current_filename = Path(filepath).name
self.last_file_path = filepath # Store the full path for later cleanup
if Path(filepath) not in self.download_files and Path(filepath).suffix.lower() not in SUBTITLE_EXTENSIONS:
self._media_phase += 1
self.download_files.add(Path(filepath))
logger.debug(f"Extracted filename: {self.current_filename}") # DEBUG
@@ -755,6 +760,11 @@ class DownloadThread(QThread):
if percent_match:
try:
percent = float(percent_match.group(1))
# When video and audio download as separate streams, scale each
# phase into its share of the bar instead of jumping 0-100 twice
if self.expected_phases > 1 and not self.is_playlist:
completed = max(0, min(self._media_phase - 1, self.expected_phases - 1))
percent = (completed * 100.0 + percent) / self.expected_phases
self.progress_signal.emit(percent)
except (ValueError, IndexError):
pass
+2 -2
View File
@@ -91,7 +91,7 @@ def get_ffmpeg_install_path() -> Path:
For Windows, tries to find the latest essentials build dynamically.
"""
if OS_NAME == "Windows":
ffmpeg_base = Path(os.getenv("LOCALAPPDATA")) / "ffmpeg" # type: ignore
ffmpeg_base = Path(os.getenv("LOCALAPPDATA", str(Path.home() / "AppData" / "Local"))) / "ffmpeg"
# If the directory exists, look for any ffmpeg-*-essentials_build folder
if ffmpeg_base.exists():
@@ -205,7 +205,7 @@ def install_ffmpeg_windows(progress_callback=None) -> bool:
try:
# Define variables for essentials build
extract_dir = Path(os.getenv("LOCALAPPDATA")) / "ffmpeg" # type: ignore
extract_dir = Path(os.getenv("LOCALAPPDATA", str(Path.home() / "AppData" / "Local"))) / "ffmpeg"
# Create extraction directory if it doesn't exist
extract_dir.mkdir(exist_ok=True)
+5 -2
View File
@@ -72,8 +72,11 @@ def should_refresh_cache(tool_name: str, current_path: Optional[str]) -> bool:
if not cache.get("version"):
return True
# Refresh if path changed
if cache.get("path") != current_path:
# Refresh if path changed (cache stores str; callers may pass Path -
# compare normalized strings or the cache would never hit)
cached_path = cache.get("path")
normalized_current = str(current_path) if current_path is not None else None
if cached_path != normalized_current:
return True
# Refresh if file was modified
@@ -3,6 +3,8 @@ Custom functionality dialogs for YTSage application.
Contains dialogs for custom commands, cookies, time ranges, and other special features.
"""
import os
import shlex
import subprocess
import threading
from pathlib import Path
@@ -32,7 +34,7 @@ from PySide6.QtWidgets import (
from ..ytsage_smooth_tab_widget import SmoothTabWidget
from ...core.ytsage_yt_dlp import get_yt_dlp_path
from ...core.ytsage_utils import update_auto_update_settings
from ...utils.ytsage_constants import YTDLP_DOCS_URL
from ...utils.ytsage_constants import SUBPROCESS_CREATIONFLAGS, YTDLP_DOCS_URL
from ...utils.ytsage_config_manager import ConfigManager
from ...utils.ytsage_localization import LocalizationManager, _
from ...utils.ytsage_logger import logger
@@ -55,16 +57,28 @@ class CommandWorker(QObject):
self.command = command
self.url = url
self.path = path
self._proc = None
self._cancelled = False
def cancel(self):
"""Terminate a running command."""
self._cancelled = True
if self._proc and self._proc.poll() is None:
try:
self._proc.terminate()
except Exception:
pass
def run_command(self):
"""Run the yt-dlp command and emit signals for output"""
try:
# Split command into arguments
args = self.command.split()
# Split command into arguments; posix=False on Windows so quoted
# backslash paths like "C:\Users\..." survive intact
args = shlex.split(self.command, posix=(os.name != "nt"))
# Build the full command
yt_dlp_path = get_yt_dlp_path()
base_cmd = [yt_dlp_path] + args
base_cmd = [str(yt_dlp_path)] + args
# Add download path if specified
if self.path:
@@ -78,19 +92,24 @@ class CommandWorker(QObject):
self.output_received.emit("=" * 50)
# Run the command
proc = subprocess.Popen(
self._proc = subprocess.Popen(
base_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
encoding="utf-8",
errors="replace",
creationflags=SUBPROCESS_CREATIONFLAGS,
)
proc = self._proc
# Stream output
for line in proc.stdout: # type: ignore[reportOptionalIterable]
if line.strip(): # Only show non-empty lines
self.output_received.emit(line.rstrip())
with proc.stdout: # type: ignore[union-attr]
for line in proc.stdout: # type: ignore[reportOptionalIterable]
if self._cancelled:
break
if line.strip(): # Only show non-empty lines
self.output_received.emit(line.rstrip())
ret = proc.wait()
self.output_received.emit("=" * 50)
@@ -985,5 +985,5 @@ class AutoUpdateSettingsDialog(QDialog):
msg_box.exec()
except Exception as e:
logger.exception(f"Error saving auto-update settings: {e}")
msg_box = self._create_styled_message_box(QMessageBox.Icon.Critical, _("settings.error_title"), _("settings", "error_saving", error=str(e)))
msg_box = self._create_styled_message_box(QMessageBox.Icon.Critical, _("settings.error_title"), _("settings.error_saving", error=str(e)))
msg_box.exec()
+4 -1
View File
@@ -346,7 +346,10 @@ class FormatTableMixin:
checkbox.setStyleSheet("QCheckBox { margin-left: 8px; }")
checkbox.format_id = f["format_id"]
checkbox.is_audio_only = f.get("vcodec") == "none"
checkbox.has_audio = f.get("acodec") != "none"
# A missing acodec means unknown, not progressive - treating it as
# has-audio skips the +bestaudio merge and yields silent videos
acodec = f.get("acodec")
checkbox.has_audio = acodec is not None and acodec != "none"
checkbox.clicked.connect(lambda checked, cb=checkbox: self.handle_checkbox_click(cb))
self.format_checkboxes.append(checkbox)