diff --git a/bragi.py b/bragi.py index 3552134..2ae5152 100755 --- a/bragi.py +++ b/bragi.py @@ -510,6 +510,8 @@ class MumbleBot: uri = music_wrapper.uri() self.log.info("bot: play music " + music_wrapper.format_debug_string()) + self.last_ffmpeg_err = "" + self.ffmpeg_fatal_error = "" # Only announce if configured and not a silent item (e.g., join sounds) if var.config.getboolean('bot', 'announce_current_music') and not music_wrapper.silent: @@ -643,8 +645,10 @@ class MumbleBot: self._loop_status = f'Wait for buffer {self.mumble.sound_output.get_buffer_size():.3f}' time.sleep(0.01) + had_ffmpeg_thread = self.thread is not None + ffmpeg_returncode = None raw_music = None - if self.thread: + if had_ffmpeg_thread: # I get raw from ffmpeg thread # move playhead forward self._loop_status = 'Reading raw' @@ -678,15 +682,20 @@ class MumbleBot: time.sleep(0.1) self.on_interrupting = False else: + ffmpeg_returncode = self.thread.poll() time.sleep(0.1) else: time.sleep(0.1) if not self.is_pause and not raw_music: - self._cleanup_ffmpeg_process() + cleanup_returncode = self._cleanup_ffmpeg_process() + if ffmpeg_returncode is None: + ffmpeg_returncode = cleanup_returncode # bot is not paused, but ffmpeg thread has gone. # indicate that last song has finished, or the bot just resumed from pause, or something is wrong. - if self.read_pcm_size < self.pcm_buffer_size \ + if had_ffmpeg_thread \ + and ffmpeg_returncode not in (None, 0) \ + and self.read_pcm_size < self.pcm_buffer_size \ and var.playlist.current_index != -1 \ and self.last_ffmpeg_err: current = var.playlist.current_item() @@ -884,6 +893,7 @@ class MumbleBot: def _cleanup_ffmpeg_process(self): """Properly cleanup FFmpeg process and associated resources""" + returncode = None if self.thread: try: # Check if process is already terminated to avoid double cleanup @@ -898,6 +908,7 @@ class MumbleBot: else: # Process already terminated, just wait to reap it self.thread.wait() + returncode = self.thread.returncode except Exception as e: self.log.error(f"bot: Error cleaning up FFmpeg process: {e}") finally: @@ -909,6 +920,7 @@ class MumbleBot: except Exception as e: self.log.error(f"bot: Error closing stderr pipe: {e}") self.thread = None + return returncode def interrupt(self): # Kill the ffmpeg thread diff --git a/tests/test_ffmpeg_http_errors.py b/tests/test_ffmpeg_http_errors.py index c01094d..dd38f74 100644 --- a/tests/test_ffmpeg_http_errors.py +++ b/tests/test_ffmpeg_http_errors.py @@ -19,6 +19,12 @@ class FakeCache: class FakeItem: id = "track-id" + def is_ready(self): + return True + + def is_failed(self): + return False + def format_debug_string(self): return "fake item" @@ -30,6 +36,7 @@ class FakePlaylist: def __init__(self, item=None): self.item = item self.removed_id = None + self.current_index = 0 if item else -1 def current_item(self): return self.item @@ -37,6 +44,22 @@ class FakePlaylist: def remove_by_id(self, item_id): self.removed_id = item_id self.item = None + self.current_index = -1 + + +class FakeSoundOutput: + def get_buffer_size(self): + return 0 + + +class FakeMumble: + def __init__(self): + self.sound_output = FakeSoundOutput() + self.alive_checks = 0 + + def is_alive(self): + self.alive_checks += 1 + return self.alive_checks < 2 class FfmpegHttpErrorTests(unittest.TestCase): @@ -78,6 +101,38 @@ class FfmpegHttpErrorTests(unittest.TestCase): bot.send_channel_msg.assert_called_once() self.assertEqual(bot.ffmpeg_fatal_error, "") + def test_waiting_ready_item_ignores_stale_ffmpeg_stderr_before_launch(self): + item = FakeItem() + var.playlist = FakePlaylist(item) + var.cache = FakeCache() + + bot = self.make_bot() + bot.mumble = FakeMumble() + bot.thread = None + bot.is_pause = False + bot.wait_for_ready = True + bot.exit = False + bot.read_pcm_size = 0 + bot.pcm_buffer_size = 1920 + bot.playhead = 0 + bot.song_start_at = -1 + bot._loop_status = "" + bot.last_ffmpeg_err = "[aist#0:0/pcm_s16le] Guessed Channel Layout: stereo" + bot._cleanup_ffmpeg_process = mock.Mock(return_value=None) + bot.launch_music = mock.Mock() + bot.async_download_next = mock.Mock() + bot.send_channel_msg = mock.Mock() + + with mock.patch("bragi.time.sleep", return_value=None): + bragi.MumbleBot.loop(bot) + + bot.launch_music.assert_called_once_with(item, 0) + bot.async_download_next.assert_called_once_with() + self.assertIsNone(var.playlist.removed_id) + self.assertIsNone(var.cache.freed_id) + bot.send_channel_msg.assert_not_called() + self.assertFalse(bot.wait_for_ready) + if __name__ == "__main__": unittest.main() diff --git a/util.py b/util.py index 33903c3..b3a0396 100644 --- a/util.py +++ b/util.py @@ -21,6 +21,7 @@ from sys import platform import traceback import requests from packaging import version +from urllib.parse import urlsplit, urlunsplit import yt_dlp as youtube_dl YT_PKG_NAME = 'yt-dlp' @@ -365,6 +366,28 @@ class Dir(object): # Parse the html from the message to get the URL +def _lower_url_host(netloc): + userinfo = "" + hostport = netloc + if "@" in netloc: + userinfo, hostport = netloc.rsplit("@", 1) + userinfo += "@" + + if hostport.startswith("["): + closing_bracket = hostport.find("]") + if closing_bracket != -1: + host = hostport[:closing_bracket + 1] + suffix = hostport[closing_bracket + 1:] + return userinfo + host.lower() + suffix + + if ":" in hostport: + host, separator, port = hostport.rpartition(":") + if port.isdigit(): + return userinfo + host.lower() + separator + port + + return userinfo + hostport.lower() + + def get_url_from_input(string): string = string.strip() if not (string.startswith("http") or string.startswith("HTTP")): @@ -374,14 +397,24 @@ def get_url_from_input(string): else: return "" - match = re.search(r"(http|https)://(\S*)?/(\S*)", string, flags=re.IGNORECASE) - if match: - url = match[1].lower() + "://" + match[2].lower() + "/" + match[3] - # https://github.com/mumble-voip/mumble/issues/4999 - return html.unescape(url) - else: + match = re.match(r"https?://\S+", string, flags=re.IGNORECASE) + if not match: return "" + # https://github.com/mumble-voip/mumble/issues/4999 + url = html.unescape(match.group(0)) + parsed = urlsplit(url) + if parsed.scheme.lower() not in ("http", "https") or not parsed.netloc: + return "" + + return urlunsplit(( + parsed.scheme.lower(), + _lower_url_host(parsed.netloc), + parsed.path, + parsed.query, + parsed.fragment, + )) + def youtube_search(query): global log