diff --git a/lib/gui/resolver_window.py b/lib/gui/resolver_window.py index d0585f78..d77c8654 100644 --- a/lib/gui/resolver_window.py +++ b/lib/gui/resolver_window.py @@ -28,6 +28,7 @@ def __init__( close_callback: Optional[Any] = None, is_subtitle_download: bool = False, local_subtitle_path: Optional[str] = None, + direct_playback_handoff: bool = False, ) -> None: super().__init__( xml_file, @@ -42,6 +43,8 @@ def __init__( self.pack_select: bool = False self.is_subtitle_download = is_subtitle_download self.local_subtitle_path = local_subtitle_path + self.direct_playback_handoff = direct_playback_handoff + self.playback_resolution_attempted = False self.item_information: Dict = item_information or {} self.close_callback: Optional[Any] = close_callback self.playback_info: Optional[Dict[str, Any]] = None @@ -102,6 +105,15 @@ def resolve_source(self) -> Optional[Dict[str, Any]]: if not self.playback_info: raise Exception("Failed to resolve source") + if self.direct_playback_handoff: + self.playback_info["direct_playback_handoff"] = True + kodilog( + "[RESOLVER] Original Kodi resolution already consumed; " + "using explicit Player.play()" + ) + + self.playback_resolution_attempted = True + player = JacktookPLayer( on_started=self.handle_playback_started, on_error=self.handle_playback_error, diff --git a/lib/gui/source_select.py b/lib/gui/source_select.py index f7b1f06a..c7a99c56 100644 --- a/lib/gui/source_select.py +++ b/lib/gui/source_select.py @@ -89,6 +89,10 @@ def __init__( self.filtered_sources: Optional[List[TorrentStream]] = None self.filter_applied: bool = False self.resolved = False + # Kodi's original plugin resolution handle can only be consumed once. + # If a playback attempt fails but SourceSelect stays open, subsequent + # attempts must start playback explicitly with Player.play(). + self._kodi_resolution_consumed = False def onInit(self) -> None: theme_index = get_setting("source_select_theme", "0") @@ -644,6 +648,10 @@ def _resolve_item( local_subtitle_path: Optional[str] = None, ) -> None: self.setProperty("resolving", "true") + kodi_resolution_consumed = bool( + getattr(self, "_kodi_resolution_consumed", False) + ) + resolver_window = ResolverWindow( "resolver.xml", ADDON_PATH, @@ -652,9 +660,28 @@ def _resolve_item( item_information=self.item_information, is_subtitle_download=is_subtitle_download, local_subtitle_path=local_subtitle_path, + direct_playback_handoff=kodi_resolution_consumed, ) - self.resolved = resolver_window.doModal(pack_select) - del resolver_window + try: + self.resolved = resolver_window.doModal(pack_select) + + playback_resolution_attempted = ( + getattr( + resolver_window, + "playback_resolution_attempted", + False, + ) + is True + ) + + self._kodi_resolution_consumed = ( + kodi_resolution_consumed + or playback_resolution_attempted + ) + finally: + if not self.resolved: + self.setProperty("resolving", "false") + del resolver_window def show_resume_dialog(self, playback_percent: float) -> Optional[bool]: try: diff --git a/lib/player.py b/lib/player.py index 0086dee2..1a68ea8f 100644 --- a/lib/player.py +++ b/lib/player.py @@ -47,6 +47,7 @@ AUTOPLAY_CONTEXT_NEXT_EPISODE = 1 ACTIVE_PLAYER_SESSION_PROPERTY = "jacktook_active_player_session" +ELEMENTUM_RESOLUTION_STATUS_PREFIX = "plugin.video.elementum.resolution_status." PLAYNEXT_ACTION_PROPERTY = "jacktook_next_dialog_action" total_time_errors = ("0.0", "", 0.0, None) video_fullscreen_check = "Window.IsActive(fullscreenvideo)" @@ -84,6 +85,7 @@ def __init__(self, on_started=None, on_error=None): self._trakt_playback_delete_attempted = False self.playback_session_id = "" self._was_superseded = False + self._playback_error_detected = False def _activate_playback_session(self): self.playback_session_id = uuid4().hex @@ -92,6 +94,52 @@ def _activate_playback_session(self): clear_property(PLAYNEXT_ACTION_PROPERTY) kodilog(f"[PLAYER] Activated playback session {self.playback_session_id[:8]}") + def _scope_elementum_resolution_signal(self): + if not self.url or not self.url.startswith( + "plugin://plugin.video.elementum/play" + ): + return + + session_id = getattr(self, "playback_session_id", "") + if not session_id: + kodilog("[PLAYER] Elementum resolution signal has no playback session") + return + + # Each playback gets its own failure property so late cleanup from an + # older Elementum player cannot overwrite a newer attempt. + failure_property = ( + f"{ELEMENTUM_RESOLUTION_STATUS_PREFIX}{session_id}" + ) + clear_property(failure_property) + + # A reused Elementum playback URL may already contain an older session + # marker. Replace it rather than accumulating markers across handoffs. + from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit + + parsed = urlsplit(self.url) + query = [ + (key, value) + for key, value in parse_qsl(parsed.query, keep_blank_values=True) + if key != "resolution_token" + ] + query.append(("resolution_token", session_id)) + + self.url = urlunsplit( + ( + parsed.scheme, + parsed.netloc, + parsed.path, + urlencode(query), + parsed.fragment, + ) + ) + self.data["url"] = self.url + + kodilog( + f"[PLAYER] Scoped Elementum resolution to " + f"{session_id[:8]}" + ) + def _owns_playback_session(self) -> bool: return bool(self.playback_session_id) and ( get_property(ACTIVE_PLAYER_SESSION_PROPERTY) == self.playback_session_id @@ -126,6 +174,7 @@ def run(self, data=None): data = {} self.set_constants(data) self._activate_playback_session() + self._scope_elementum_resolution_signal() self.clear_playback_properties() if not self._is_trakt_tracking_excluded(): self.add_external_trakt_scrolling() @@ -225,7 +274,7 @@ def play_video(self, list_item): # The original plugin resolution has already completed. Start # the queued external-plugin URL explicitly instead of trying # to resolve a stale addon handle. - kodilog("[PLAYER] play_video: calling Player.play for internal PlayNext handoff") + kodilog("[PLAYER] play_video: calling Player.play for explicit playback handoff") self.play(self.url, list_item) else: # Normal plugin entry point: answer Kodi's pending resolution. @@ -358,12 +407,32 @@ def handle_trakt_pause_resume(self): self._playback_was_paused = is_paused + def onPlayBackError(self): + self._playback_error_detected = True + kodilog("[PLAYER] Kodi reported playback error while resolving") + def monitor(self): ensure_dialog_closed = False kodilog("[PLAYER] monitor() entered") try: while not self.isPlayingVideo(): + elementum_failure_property = ( + f"{ELEMENTUM_RESOLUTION_STATUS_PREFIX}" + f"{self.playback_session_id}" + ) + if get_property(elementum_failure_property) == "failed": + clear_property(elementum_failure_property) + kodilog( + "[PLAYER] Elementum reported failed/cancelled resolution" + ) + self.handle_playback_failure() + return + + if self._playback_error_detected: + kodilog("[PLAYER] monitor detected failed playback resolution") + self.handle_playback_failure() + return if not self._owns_playback_session(): self._was_superseded = True kodilog("[PLAYER] monitor superseded while waiting for playback") @@ -523,9 +592,12 @@ def select_audio_stream(self): break def handle_playback_failure(self): - self.kill_dialog() if self.on_error: + # ResolverWindow owns its UI lifecycle. Let its error callback close + # only the resolver so SourceSelect remains open underneath. self.on_error() + else: + self.kill_dialog() self.stop() def handle_playback_start(self): @@ -1172,6 +1244,7 @@ def set_constants(self, data): self._simkl_resume_playback_observed = False self._simkl_playback_delete_attempted = False self._was_superseded = False + self._playback_error_detected = False from lib.utils.general.utils import extract_release_group self.preferred_group = extract_release_group(self.data.get("title", "")) diff --git a/lib/utils/player/utils.py b/lib/utils/player/utils.py index 3d317849..a247b281 100644 --- a/lib/utils/player/utils.py +++ b/lib/utils/player/utils.py @@ -249,6 +249,7 @@ def get_elementum_url( f"&show={quote(str(tmdb_id))}" f"&season={quote(str(season))}" f"&episode={quote(str(episode))}" + "&skip_file_dialog=true" ) elif mode in ("movie", "movies"): file_match = _build_elementum_movie_file_match((data or {}).get("title")) diff --git a/resources/skins/Default/1080i/source_select.xml b/resources/skins/Default/1080i/source_select.xml index b919316e..aa78358e 100644 --- a/resources/skins/Default/1080i/source_select.xml +++ b/resources/skins/Default/1080i/source_select.xml @@ -18,7 +18,6 @@ white.png 8000000 scale - Conditional !String.IsEqual(Window().Property(instant_close),true) @@ -27,7 +26,6 @@ $INFO[Window().Property(info.fanart)] FFFFFFFF scale - Conditional !String.IsEqual(Window().Property(instant_close),true) @@ -35,7 +33,6 @@ white.png CC000000 - Conditional !String.IsEqual(Window().Property(instant_close),true) @@ -47,7 +44,6 @@ WindowClose - Conditional diff --git a/tests/unit/test_elementum_skip_file_dialog.py b/tests/unit/test_elementum_skip_file_dialog.py new file mode 100644 index 00000000..ab30579c --- /dev/null +++ b/tests/unit/test_elementum_skip_file_dialog.py @@ -0,0 +1,79 @@ +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse + +import pytest + + +def _query(url: str): + return parse_qs(urlparse(url).query) + + +def test_elementum_exact_tv_episode_disables_manual_file_dialog(): + from lib.utils.player import utils + + with patch.object(utils, "is_elementum_addon", return_value=True): + url = utils.get_elementum_url( + "magnet:?xt=urn:btih:SEASONPACK", + "", + "tv", + {"tmdb_id": "2190"}, + data={ + "title": "A TV Show", + "tv_data": {"season": 2, "episode": 1}, + }, + ) + + query = _query(url) + + assert query["show"] == ["2190"] + assert query["season"] == ["2"] + assert query["episode"] == ["1"] + assert query["skip_file_dialog"] == ["true"] + + +@pytest.mark.parametrize( + ("ids", "tv_data"), + [ + ({"tmdb_id": "2190"}, {"season": 2}), + ({"tmdb_id": "2190"}, {"season": 2, "episode": None}), + ({}, {"season": 2, "episode": 1}), + ], +) +def test_elementum_incomplete_tv_metadata_keeps_existing_fallback(ids, tv_data): + from lib.utils.player import utils + + with patch.object(utils, "is_elementum_addon", return_value=True): + url = utils.get_elementum_url( + "magnet:?xt=urn:btih:EPISODE", + "", + "tv", + ids, + data={"title": "A TV Show", "tv_data": tv_data}, + ) + + query = _query(url) + + assert "skip_file_dialog" not in query + assert "show" not in query + assert "season" not in query + assert "episode" not in query + + +def test_elementum_movie_does_not_disable_manual_file_dialog(): + from lib.utils.player import utils + + with patch.object(utils, "is_elementum_addon", return_value=True): + url = utils.get_elementum_url( + "magnet:?xt=urn:btih:MOVIEPACK", + "", + "movies", + {"tmdb_id": "4271"}, + data={ + "title": "Mais où est donc passée la 7ème compagnie ?", + }, + ) + + query = _query(url) + + assert "file_match" in query + assert "skip_file_dialog" not in query diff --git a/tests/unit/test_playback_resolution_failure.py b/tests/unit/test_playback_resolution_failure.py new file mode 100644 index 00000000..8016ff31 --- /dev/null +++ b/tests/unit/test_playback_resolution_failure.py @@ -0,0 +1,263 @@ +import importlib.util +from pathlib import Path +from unittest.mock import MagicMock, patch + +import xbmc + + +def _load_real_player_module(monkeypatch): + """Load lib/player.py with a real xbmc.Player base class. + + tests/conftest.py mocks the whole xbmc module, which makes xbmc.Player a + MagicMock. JacktookPLayer needs a real class as its base for this unit test. + """ + + class PlayerStub: + pass + + monkeypatch.setattr(xbmc, "Player", PlayerStub) + + player_path = Path(__file__).resolve().parents[2] / "lib" / "player.py" + spec = importlib.util.spec_from_file_location( + "_jacktook_player_resolution_test", + player_path, + ) + + if spec is None or spec.loader is None: + raise RuntimeError("Unable to load lib/player.py") + + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_player_callback_marks_failed_playback_resolution(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player._playback_error_detected = False + + JacktookPLayer.onPlayBackError(player) + + assert player._playback_error_detected is True + + +def test_source_select_clears_resolving_after_failed_resolution(): + from lib.gui.source_select import SourceSelect + + window = object.__new__(SourceSelect) + window.resolved = False + window.item_information = {} + window.setProperty = MagicMock() + + resolver = MagicMock() + resolver.doModal.return_value = False + + with patch( + "lib.gui.source_select.ResolverWindow", + return_value=resolver, + ): + SourceSelect._resolve_item( + window, + selected_source=MagicMock(), + pack_select=False, + ) + + assert window.resolved is False + window.setProperty.assert_any_call("resolving", "true") + window.setProperty.assert_any_call("resolving", "false") + + +def test_elementum_url_is_scoped_to_current_playback_session(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player.playback_session_id = "new-session" + player.url = ( + "plugin://plugin.video.elementum/play" + "?uri=magnet%3A%3Fxt%3Durn%3Abtih%3ATEST" + "&type=tv" + "&resolution_token=old-session" + ) + player.data = {"url": player.url} + + JacktookPLayer._scope_elementum_resolution_signal(player) + + assert "resolution_token=new-session" in player.url + assert "old-session" not in player.url + assert player.data["url"] == player.url + + +def test_playback_failure_callback_keeps_parent_dialogs_open(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player.on_error = MagicMock() + player.kill_dialog = MagicMock() + player.stop = MagicMock() + + JacktookPLayer.handle_playback_failure(player) + + player.on_error.assert_called_once_with() + player.kill_dialog.assert_not_called() + player.stop.assert_called_once_with() + + +def test_playback_failure_without_callback_keeps_legacy_dialog_cleanup(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player.on_error = None + player.kill_dialog = MagicMock() + player.stop = MagicMock() + + JacktookPLayer.handle_playback_failure(player) + + player.kill_dialog.assert_called_once_with() + player.stop.assert_called_once_with() + + +def test_elementum_scope_clears_only_current_session_signal(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + clear_property = MagicMock() + monkeypatch.setattr(player_module, "clear_property", clear_property) + + player = object.__new__(JacktookPLayer) + player.playback_session_id = "session-two" + player.url = ( + "plugin://plugin.video.elementum/play" + "?uri=magnet%3A%3Fxt%3Durn%3Abtih%3ATEST" + ) + player.data = {"url": player.url} + + JacktookPLayer._scope_elementum_resolution_signal(player) + + clear_property.assert_called_once_with( + "plugin.video.elementum.resolution_status.session-two" + ) + + +def test_source_select_backgrounds_do_not_fade_while_resolving(): + import re + + xml = ( + Path(__file__).resolve().parents[2] + / "resources" + / "skins" + / "Default" + / "1080i" + / "source_select.xml" + ).read_text() + + resolving_fades = re.findall( + r']*effect="fade"[^>]*' + r'condition="String\.IsEqual' + r'\(Window\(\)\.Property\(resolving\),true\)"' + r'[^>]*>Conditional', + xml, + ) + + assert resolving_fades == [] + + +def test_source_select_retry_uses_explicit_playback_handoff(): + from lib.gui.source_select import SourceSelect + + window = object.__new__(SourceSelect) + window.resolved = False + window.item_information = {} + window.setProperty = MagicMock() + window._kodi_resolution_consumed = False + + first_resolver = MagicMock() + first_resolver.doModal.return_value = False + first_resolver.playback_resolution_attempted = True + + retry_resolver = MagicMock() + retry_resolver.doModal.return_value = False + retry_resolver.playback_resolution_attempted = True + + with patch( + "lib.gui.source_select.ResolverWindow", + side_effect=[first_resolver, retry_resolver], + ) as resolver_class: + SourceSelect._resolve_item( + window, + selected_source=MagicMock(), + pack_select=False, + ) + + assert ( + resolver_class.call_args_list[0] + .kwargs["direct_playback_handoff"] + is False + ) + assert window._kodi_resolution_consumed is True + + SourceSelect._resolve_item( + window, + selected_source=MagicMock(), + pack_select=False, + ) + + assert ( + resolver_class.call_args_list[1] + .kwargs["direct_playback_handoff"] + is True + ) + + +def test_normal_playback_uses_pending_kodi_resolution(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player.url = "plugin://plugin.video.elementum/play?uri=test" + player.data = {} + player.play = MagicMock() + player._check_volume = MagicMock(return_value=True) + player._handle_trakt_scrobble = MagicMock() + player._handle_simkl_scrobble = MagicMock() + player.handle_subtitles = MagicMock() + player.monitor = MagicMock() + + list_item = MagicMock() + + with patch.object(player_module, "setResolvedUrl") as set_resolved: + JacktookPLayer.play_video(player, list_item) + + set_resolved.assert_called_once_with( + player_module.ADDON_HANDLE, + True, + list_item, + ) + player.play.assert_not_called() + + +def test_explicit_handoff_uses_player_play(monkeypatch): + player_module = _load_real_player_module(monkeypatch) + JacktookPLayer = player_module.JacktookPLayer + + player = object.__new__(JacktookPLayer) + player.url = "plugin://plugin.video.elementum/play?uri=test" + player.data = {"direct_playback_handoff": True} + player.play = MagicMock() + player._check_volume = MagicMock(return_value=True) + player._handle_trakt_scrobble = MagicMock() + player._handle_simkl_scrobble = MagicMock() + player.handle_subtitles = MagicMock() + player.monitor = MagicMock() + + list_item = MagicMock() + + with patch.object(player_module, "setResolvedUrl") as set_resolved: + JacktookPLayer.play_video(player, list_item) + + set_resolved.assert_not_called() + player.play.assert_called_once_with(player.url, list_item)