Address PR review feedback

- Use environment variables for Windows tool detection instead of
  scanning all drives (LOCALAPPDATA, PROGRAMFILES, PROGRAMFILES(X86))
- Remove pgsrip_path config field and use pgsrip Python API directly
- Update dependency checks to use importlib for pgsrip library
- Fix BabelLanguage to handle both 2-letter and 3-letter ISO codes
- Update error messages and installation instructions

All changes pass pre-commit linting checks.

Author: mikeSGman

fastflix/models/config.py

@@ -111,22 +111,29 @@ def find_ocr_tool(name):
 
     # Special handling for tesseract on Windows (not in PATH by default)
     if name == "tesseract" and win_based:
-        # Check common install locations on all drives
-        import string
-        drives = [f"{d}:" for d in string.ascii_uppercase if Path(f"{d}:/").exists()]
-
-        for drive in drives:
-            common_paths = [
-                Path(f"{drive}/Program Files/Tesseract-OCR/tesseract.exe"),
-                Path(f"{drive}/Program Files (x86)/Tesseract-OCR/tesseract.exe"),
-            ]
-            for path in common_paths:
-                if path.exists():
-                    return path
+        # Check common install locations using environment variables
+        localappdata = os.getenv("LOCALAPPDATA")
+        program_files = os.getenv("PROGRAMFILES")
+        program_files_x86 = os.getenv("PROGRAMFILES(X86)")
+
+        common_paths = []
+        # Check user-local installation first
+        if localappdata:
+            common_paths.append(Path(localappdata) / "Programs" / "Tesseract-OCR" / "tesseract.exe")
+        # Check system-wide installations
+        if program_files:
+            common_paths.append(Path(program_files) / "Tesseract-OCR" / "tesseract.exe")
+        if program_files_x86:
+            common_paths.append(Path(program_files_x86) / "Tesseract-OCR" / "tesseract.exe")
+
+        for path in common_paths:
+            if path.exists():
+                return path
 
         # Check Windows registry for Tesseract install location
         try:
             import winreg
+
             # Try HKEY_LOCAL_MACHINE first (system-wide install)
             for root_key in [winreg.HKEY_LOCAL_MACHINE, winreg.HKEY_CURRENT_USER]:
                 try:
@@ -143,17 +150,24 @@ def find_ocr_tool(name):
 
     # Special handling for mkvmerge on Windows
     if name == "mkvmerge" and win_based:
-        import string
-        drives = [f"{d}:" for d in string.ascii_uppercase if Path(f"{d}:/").exists()]
-
-        for drive in drives:
-            common_paths = [
-                Path(f"{drive}/Program Files/MKVToolNix/mkvmerge.exe"),
-                Path(f"{drive}/Program Files (x86)/MKVToolNix/mkvmerge.exe"),
-            ]
-            for path in common_paths:
-                if path.exists():
-                    return path
+        # Check common install locations using environment variables
+        localappdata = os.getenv("LOCALAPPDATA")
+        program_files = os.getenv("PROGRAMFILES")
+        program_files_x86 = os.getenv("PROGRAMFILES(X86)")
+
+        common_paths = []
+        # Check user-local installation first
+        if localappdata:
+            common_paths.append(Path(localappdata) / "Programs" / "MKVToolNix" / "mkvmerge.exe")
+        # Check system-wide installations
+        if program_files:
+            common_paths.append(Path(program_files) / "MKVToolNix" / "mkvmerge.exe")
+        if program_files_x86:
+            common_paths.append(Path(program_files_x86) / "MKVToolNix" / "mkvmerge.exe")
+
+        for path in common_paths:
+            if path.exists():
+                return path
 
     # Check in FastFlix OCR tools folder
     ocr_folder = Path(user_data_dir("FastFlix_OCR", appauthor=False, roaming=True))
@@ -243,7 +257,6 @@ class Config(BaseModel):
     enable_pgs_ocr: bool = False
     tesseract_path: Path | None = Field(default_factory=lambda: find_ocr_tool("tesseract"))
     mkvmerge_path: Path | None = Field(default_factory=lambda: find_ocr_tool("mkvmerge"))
-    pgsrip_path: Path | None = Field(default_factory=lambda: find_ocr_tool("pgsrip"))
     pgs_ocr_language: str = "eng"
 
     def encoder_opt(self, profile_name, profile_option_name):

fastflix/widgets/background_tasks.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
+import importlib.util
 import logging
 import os
-import shutil
 from pathlib import Path
 from subprocess import PIPE, STDOUT, Popen, run, check_output
 from packaging import version
@@ -190,31 +190,30 @@ def _check_pgsrip_dependencies(self) -> bool:
         if not self.app.fastflix.config.mkvmerge_path:
             missing.append("mkvtoolnix")
 
-        # Check pgsrip
-        if not self.app.fastflix.config.pgsrip_path:
-            missing.append("pgsrip")
+        # Check if pgsrip Python library is available
+        if importlib.util.find_spec("pgsrip") is None:
+            missing.append("pgsrip (Python library)")
 
         if missing:
             self.main.thread_logging_signal.emit(
                 f"ERROR:{t('Missing dependencies for PGS OCR')}: {', '.join(missing)}\n\n"
                 f"Install instructions:\n"
-                f"  Windows: Run setup_pgs_ocr_windows.bat in FastFlix folder\n"
-                f"  Linux: sudo apt install tesseract-ocr mkvtoolnix && pip install pgsrip\n"
-                f"  macOS: brew install tesseract mkvtoolnix && pip install pgsrip\n\n"
-                f"Or download manually:\n"
-                f"  Tesseract: https://github.com/UB-Mannheim/tesseract/wiki\n"
-                f"  MKVToolNix: https://mkvtoolnix.download/downloads.html\n"
-                f"  pgsrip: pip install pgsrip"
+                f"  pgsrip: pip install pgsrip\n"
+                f"  Linux: sudo apt install tesseract-ocr mkvtoolnix\n"
+                f"  macOS: brew install tesseract mkvtoolnix\n"
+                f"  Windows:\n"
+                f"    - Tesseract: https://github.com/UB-Mannheim/tesseract/wiki\n"
+                f"    - MKVToolNix: https://mkvtoolnix.download/downloads.html"
             )
             return False
 
         return True
 
     def _convert_sup_to_srt(self, sup_filepath: str) -> bool:
-        """Convert an already-extracted .sup file to .srt using pgsrip OCR
+        """Convert PGS subtitle to .srt using pgsrip OCR by processing the original MKV
 
         Args:
-            sup_filepath: Path to the extracted .sup file
+            sup_filepath: Path to the extracted .sup file (used for naming output)
 
         Returns:
             True if conversion successful, False otherwise
@@ -228,80 +227,87 @@ def _convert_sup_to_srt(self, sup_filepath: str) -> bool:
                 f"INFO:{t('Converting .sup to .srt using OCR')} (this may take 3-5 minutes)..."
             )
 
-            # Convert 3-letter language code to 2-letter for pgsrip
-            # pgsrip uses 2-letter codes in filenames (e.g., "en" not "eng")
-            from fastflix.language import Language
-            try:
-                lang_2letter = Language(self.language).pt1  # Convert eng -> en
-            except:
-                lang_2letter = "en"  # Default to English if conversion fails
-
-            # Rename .sup file to use 2-letter language code (what pgsrip expects)
-            sup_path = Path(sup_filepath)
-            if f".{self.language}." in sup_path.name:
-                # Replace 3-letter with 2-letter in filename
-                new_name = sup_path.name.replace(f".{self.language}.", f".{lang_2letter}.")
-                new_sup_path = sup_path.parent / new_name
-                sup_path.rename(new_sup_path)
-                sup_filepath = str(new_sup_path)
-
-            # Run pgsrip on the already-extracted .sup file
-            pgsrip_cmd = str(self.app.fastflix.config.pgsrip_path) if self.app.fastflix.config.pgsrip_path else "pgsrip"
+            # Import pgsrip Python API
+            from pgsrip import pgsrip, Mkv, Options
+            from babelfish import Language as BabelLanguage
 
             # Set environment variables for pgsrip to find tesseract
-            import os
-            env = os.environ.copy()
             if self.app.fastflix.config.tesseract_path:
                 # Add tesseract directory to PATH so pytesseract can find it
                 tesseract_dir = str(Path(self.app.fastflix.config.tesseract_path).parent)
-                env['PATH'] = f"{tesseract_dir}{os.pathsep}{env.get('PATH', '')}"
-                env['TESSERACT_CMD'] = str(self.app.fastflix.config.tesseract_path)
+                os.environ["PATH"] = f"{tesseract_dir}{os.pathsep}{os.environ.get('PATH', '')}"
+                os.environ["TESSERACT_CMD"] = str(self.app.fastflix.config.tesseract_path)
 
-            pgsrip_result = run(
-                [
-                    pgsrip_cmd,
-                    "--language", lang_2letter,  # Use 2-letter code (e.g., "en", "es", "fr")
-                    "--force",                    # Overwrite existing files
-                    sup_filepath
-                ],
-                capture_output=True,
-                text=True,
-                timeout=600,  # 10 minute timeout for OCR
-                env=env  # Pass environment with TESSERACT_CMD
-            )
+            # pgsrip needs the original MKV file, not the extracted .sup
+            # The .sup file is a raw subtitle stream, but pgsrip expects an MKV container
+            sup_path = Path(sup_filepath)
+            media = Mkv(self.main.input_video)
+
+            # Configure options for pgsrip
+            # BabelLanguage needs different constructors for 2-letter vs 3-letter codes
+            try:
+                # Detect if language code is 2-letter or 3-letter
+                if len(self.language) == 2:
+                    babel_lang = BabelLanguage.fromalpha2(self.language)
+                elif len(self.language) == 3:
+                    babel_lang = BabelLanguage(self.language)
+                else:
+                    # Try as language name
+                    babel_lang = BabelLanguage.fromname(self.language)
+
+                options = Options(
+                    languages={babel_lang},
+                    overwrite=True,  # Overwrite existing .srt files
+                    one_per_lang=True,  # Create one .srt per language
+                )
+            except Exception:
+                # Fallback to English if language code is invalid
+                options = Options(
+                    languages={BabelLanguage("eng")},
+                    overwrite=True,
+                    one_per_lang=True,
+                )
 
-            if pgsrip_result.returncode != 0:
-                error_msg = pgsrip_result.stderr if pgsrip_result.stderr else pgsrip_result.stdout
-                raise Exception(f"pgsrip failed with return code {pgsrip_result.returncode}: {error_msg}")
+            # Run pgsrip conversion using Python API on the original MKV
+            # This will create .srt files in the same directory as the video
+            pgsrip.rip(media, options)
 
-            # pgsrip creates .srt file in same directory as .sup file
-            sup_path = Path(sup_filepath)
-            expected_srt = sup_path.with_suffix('.srt')
+            # Look for the created .srt file
+            # pgsrip creates files with pattern: basename.language.srt
+            video_path = Path(self.main.input_video)
+            srt_pattern = f"{video_path.stem}.*.srt"
+            srt_files = list(video_path.parent.glob(srt_pattern))
 
-            if not expected_srt.exists():
-                # Look for any .srt file created near the .sup
-                srt_files = list(sup_path.parent.glob("*.srt"))
-                if not srt_files:
-                    raise Exception(f"pgsrip completed but no .srt file found in {sup_path.parent}")
-                expected_srt = srt_files[0]
+            if not srt_files:
+                # Fallback: look for any .srt in the video directory
+                srt_files = list(video_path.parent.glob(f"{video_path.stem}.srt"))
 
-            self.main.thread_logging_signal.emit(
-                f"INFO:{t('OCR conversion successful')}: {expected_srt.name}"
-            )
+            if not srt_files:
+                raise Exception(f"pgsrip completed but no .srt file found in {video_path.parent}")
+
+            # Move the .srt file to the expected location (same dir as .sup was)
+            created_srt = srt_files[0]
+            expected_srt = sup_path.with_suffix(".srt")
+
+            if created_srt != expected_srt:
+                # Move/rename to expected location
+                import shutil
+
+                shutil.move(str(created_srt), str(expected_srt))
+
+            self.main.thread_logging_signal.emit(f"INFO:{t('OCR conversion successful')}: {expected_srt.name}")
 
             # Optionally delete the .sup file since we have .srt now
             try:
                 sup_path.unlink()
                 self.main.thread_logging_signal.emit(f"INFO:{t('Removed .sup file, kept .srt')}")
-            except:
+            except Exception:
                 pass
 
             return True
 
         except Exception as err:
-            self.main.thread_logging_signal.emit(
-                f"ERROR:{t('OCR conversion failed')}: {err}"
-            )
+            self.main.thread_logging_signal.emit(f"ERROR:{t('OCR conversion failed')}: {err}")
             return False
 
 

fastflix/widgets/panels/subtitle_panel.py

@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
+import importlib.util
 from typing import Union
 
 from box import Box
@@ -116,17 +117,23 @@ def __init__(self, app, parent, index, enabled=True, first=False):
             extract_menu.addAction(t("Extract as .sup (image - fast)"), lambda: self.extract(use_ocr=False))
 
             # Check if OCR dependencies are available
-            ocr_action = extract_menu.addAction(t("Convert to .srt (OCR - 3-5 min)"), lambda: self.extract(use_ocr=True))
+            ocr_action = extract_menu.addAction(
+                t("Convert to .srt (OCR - 3-5 min)"), lambda: self.extract(use_ocr=True)
+            )
 
             # Enable OCR option only if user enabled it AND dependencies are available
             if not self.app.fastflix.config.enable_pgs_ocr:
                 ocr_action.setEnabled(False)
                 ocr_action.setToolTip(t("Enable in Settings > 'Enable PGS to SRT OCR conversion'"))
-            elif not (self.app.fastflix.config.tesseract_path and
-                      self.app.fastflix.config.mkvmerge_path and
-                      self.app.fastflix.config.pgsrip_path):
-                ocr_action.setEnabled(False)
-                ocr_action.setToolTip(t("Missing dependencies: tesseract, mkvtoolnix, or pgsrip"))
+            else:
+                # Check if pgsrip Python library is available
+                pgsrip_ok = importlib.util.find_spec("pgsrip") is not None
+
+                if not (
+                    self.app.fastflix.config.tesseract_path and self.app.fastflix.config.mkvmerge_path and pgsrip_ok
+                ):
+                    ocr_action.setEnabled(False)
+                    ocr_action.setToolTip(t("Missing dependencies: tesseract, mkvtoolnix, or pgsrip"))
 
             self.widgets.extract.setMenu(extract_menu)
         else:
@@ -193,8 +200,12 @@ def init_move_buttons(self):
 
     def extract(self, use_ocr=False):
         worker = ExtractSubtitleSRT(
-            self.parent.app, self.parent.main, self.index, self.extract_completed_signal,
-            language=self.language, use_ocr=use_ocr
+            self.parent.app,
+            self.parent.main,
+            self.index,
+            self.extract_completed_signal,
+            language=self.language,
+            use_ocr=use_ocr,
         )
         worker.start()
         self.gif_label.show()

fastflix/widgets/settings.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 
+import importlib.util
 import logging
 import shutil
 from pathlib import Path
@@ -273,8 +274,7 @@ def in_dir():
         self.enable_pgs_ocr = QtWidgets.QCheckBox(t("Enable PGS to SRT OCR conversion"))
         self.enable_pgs_ocr.setChecked(self.app.fastflix.config.enable_pgs_ocr)
         self.enable_pgs_ocr.setToolTip(
-            t("Convert image-based PGS subtitles to text SRT using OCR.\n"
-              "Typically takes 3-5 minutes per movie.")
+            t("Convert image-based PGS subtitles to text SRT using OCR.\nTypically takes 3-5 minutes per movie.")
         )
 
         # Dependency status
@@ -314,7 +314,9 @@ def update_ocr_dependency_status(self):
         # Use config paths which use find_ocr_tool() - handles non-PATH locations
         tesseract_ok = self.app.fastflix.config.tesseract_path is not None
         mkvmerge_ok = self.app.fastflix.config.mkvmerge_path is not None
-        pgsrip_ok = self.app.fastflix.config.pgsrip_path is not None
+
+        # Check if pgsrip Python library is available
+        pgsrip_ok = importlib.util.find_spec("pgsrip") is not None
 
         status_parts = []
         status_parts.append("✓ tesseract" if tesseract_ok else "✗ tesseract")
@@ -327,7 +329,7 @@ def update_ocr_dependency_status(self):
             status_text += "\n" + link(
                 "https://github.com/cdgriffith/FastFlix/wiki/PGS-OCR-Setup",
                 "Click here for installation instructions",
-                self.app.fastflix.config.theme
+                self.app.fastflix.config.theme,
             )
 
         self.ocr_status_label.setText(status_text)