diff --git a/lib/streaming_providers/base/utils/mpd_cache.py b/lib/streaming_providers/base/utils/mpd_cache.py index 09dee38..ba32ba8 100644 --- a/lib/streaming_providers/base/utils/mpd_cache.py +++ b/lib/streaming_providers/base/utils/mpd_cache.py @@ -76,8 +76,17 @@ class MPDCacheManager: logger.debug( f"Cache expired for {cache_key} (expired {staleness}s ago)" ) - self.vfs.delete(manifest_file) - self.vfs.delete(meta_file) + # Deliberately NOT deleting the expired files here. This get() + # call may have been made with the default max_stale=0 (a + # normal cache lookup), but a *different* caller further down + # the request chain may retry with max_stale>0 as a + # last-resort fallback after a live refetch fails. If we + # delete on every plain expiry check, that fallback caller + # always finds nothing — the fallback only ever worked when + # it happened to be the first caller to see the expired + # entry. Expired entries are reaped on their own schedule by + # clear_expired() instead, so both call patterns keep working + # regardless of ordering. return None manifest_content = self.vfs.read_text(manifest_file) @@ -264,4 +273,4 @@ class MPDCacheManager: return meta except Exception as e: logger.debug(f"Error getting cache info for {cache_key}: {e}") - return None + return None \ No newline at end of file diff --git a/lib/streaming_providers/base/utils/mpd_rewriter.py b/lib/streaming_providers/base/utils/mpd_rewriter.py index 1871269..847b05b 100644 --- a/lib/streaming_providers/base/utils/mpd_rewriter.py +++ b/lib/streaming_providers/base/utils/mpd_rewriter.py @@ -7,6 +7,7 @@ Handles URL proxying, DRM key injection, quality filtering, and representation b import base64 import struct import xml.etree.ElementTree as ET +from dataclasses import dataclass from typing import Optional, Tuple, Set, Dict from urllib.parse import urljoin, quote, urlencode from datetime import datetime, timezone @@ -18,6 +19,31 @@ from .drm_key_manager import KeyConfiguration from .representation_blocklist import RepresentationBlocklist from .video_quality import VideoQualityFilter, VideoRepresentation from .time_utils import parse_iso_duration + + +@dataclass +class _RewriteState: + """ + Per-node traversal state for MPDRewriter._rewrite_node. + + This used to be five separate positional parameters threaded through + every recursive call by hand. That's how the $RepresentationID$ + substitution bug happened: current_rep_id was tracked as a local + variable in _rewrite_node but never added to the recursive call's + argument list, so it silently reset to None on every recursion step — + meaning it only ever "worked" for attributes living directly on the + element itself, never for the child + where media/initialization templates actually live in practice. + + Bundling the state into one object closes off that whole class of bug: + a recursive call either forwards the state object or it doesn't compile + (there's no way to forward "most of" a dataclass instance by accident). + """ + base_url: str + period_id: str = "" + encrypted: bool = False + kid: Optional[str] = None + rep_id: Optional[str] = None from ..models.drm.constants import DRM_SYSTEM_NAMES @@ -188,8 +214,9 @@ class MPDRewriter: raise ValueError("No AdaptationSets remain after key filtering - manifest would be empty") # Rewrite URLs with appropriate keys and context-aware base URLs - self._rewrite_node(root, mpd_base_url, encrypted_ids, as_id_to_kid, base_url_map, - False, None, "", best_video_info) + self._rewrite_node( + root, _RewriteState(base_url=mpd_base_url), encrypted_ids, as_id_to_kid, base_url_map + ) rewritten = ET.tostring(root, encoding="unicode", method="xml") if not rewritten.startswith(" str: diff --git a/lib/streaming_providers/base/utils/vfs.py b/lib/streaming_providers/base/utils/vfs.py index 4df930a..1ee758c 100644 --- a/lib/streaming_providers/base/utils/vfs.py +++ b/lib/streaming_providers/base/utils/vfs.py @@ -6,6 +6,7 @@ Provides transparent file operations for both Kodi and regular Python environmen import json import os +import threading from typing import Any, Dict, List, Optional, Tuple # Import centralized environment manager @@ -65,7 +66,9 @@ class VFS: logger.info(f"Base path from environment: {self._base_path}") - # Ensure base directory exists + # Ensure base directory exists. self._base_path is already assigned + # above, so the join_path() call inside mkdirs() will not recurse + # back into this property. self.mkdirs("") return self._base_path @@ -346,7 +349,7 @@ class VFS: pattern: File pattern filter (basic glob patterns) Returns: - List of filenames + List of filenames (basenames only, files only — not subdirectories) """ try: if not dirpath: @@ -384,6 +387,26 @@ class VFS: logger.error(f"Error listing files in {dirpath}: {e}") return [] + def listdir(self, dirpath: str = "") -> List[str]: + """ + Alias for list_files(dirpath) with the default "*" pattern. + + Several call sites (e.g. MPDCacheManager.clear_all / + clear_expired) call self.vfs.listdir() rather than + self.vfs.list_files() — this previously did not exist on VFS at + all and raised AttributeError on every call, which was silently + swallowed by those callers' broad except blocks. Kept as an + explicit method (not just a `listdir = list_files` assignment) + so it shows up in stack traces under its own name. + + Args: + dirpath: Directory path to list (relative to base_path) + + Returns: + List of filenames (basenames only, files only) + """ + return self.list_files(dirpath) + def get_size(self, filepath: str) -> Optional[int]: """ Get file size in bytes @@ -471,8 +494,14 @@ class VFS: return info -# Cache for VFS instances with different configurations +# Cache for VFS instances with different configurations. +# Guarded by a lock: this backend runs multi-threaded (Bottle), and without +# it two threads racing on the same not-yet-cached key could each construct +# and briefly use their own VFS instance for the same directory. Functionally +# harmless (both point at the same base_path), but it defeats the point of +# caching and is cheap to close off properly. _vfs_cache: Dict[Tuple[Optional[str], str], "VFS"] = {} +_vfs_cache_lock = threading.Lock() def get_vfs(config_dir: Optional[str] = None, addon_subdir: str = "") -> "VFS": @@ -488,14 +517,21 @@ def get_vfs(config_dir: Optional[str] = None, addon_subdir: str = "") -> "VFS": Returns: VFS instance """ - global _vfs_cache - cache_key = (config_dir, addon_subdir) - if cache_key not in _vfs_cache: - _vfs_cache[cache_key] = VFS(config_dir, addon_subdir) + # Fast path without the lock for the common case (already cached). + vfs = _vfs_cache.get(cache_key) + if vfs is not None: + return vfs - return _vfs_cache[cache_key] + with _vfs_cache_lock: + # Re-check inside the lock in case another thread created it while + # we were waiting. + vfs = _vfs_cache.get(cache_key) + if vfs is None: + vfs = VFS(config_dir, addon_subdir) + _vfs_cache[cache_key] = vfs + return vfs # For backward compatibility with existing code