From 2ef1857640edd9892e69c8b6a9b53b3258f4227f Mon Sep 17 00:00:00 2001 From: Neraste Date: Sun, 2 Aug 2026 18:27:45 +0200 Subject: [PATCH 1/4] Securize the creation of the VLC instance --- CHANGELOG.md | 4 ++++ src/dakara_player/media_player/vlc.py | 14 ++++++++++++-- tests/integration/test_media_player_vlc.py | 20 ++++++++++++++++++-- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3f0859..2d36a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ ## Unreleased +### Added + +- When using VLC, the player will properly crash if unexpected instance parameters are given. + ### Changed - The discovery of instrumental file or track is now delegated to the feeder, which means that this data is given by the server. There are no re-discovering processes, but some checks are performed (whether the instrumental file exists, and if the instrumental track exists). diff --git a/src/dakara_player/media_player/vlc.py b/src/dakara_player/media_player/vlc.py index 8cbecff..7a0dbed 100644 --- a/src/dakara_player/media_player/vlc.py +++ b/src/dakara_player/media_player/vlc.py @@ -123,8 +123,14 @@ def init_player(self, config, tempdir): ) # VLC objects - self.instance = vlc.Instance(config_vlc.get("instance_parameters") or []) - self.player = self.instance.media_player_new() + instance = vlc.Instance(config_vlc.get("instance_parameters") or []) + if instance is None: + raise UnexpectedInstanceParameterError("Unexpected instance parameter") + + self.instance = instance + + player = self.instance.media_player_new() + self.player = player self.event_manager = self.player.event_manager() # vlc callbacks @@ -804,3 +810,7 @@ def __init__(self, *args, track_id_audio=None, **kwargs): class VlcTooOldError(DakaraError): """Error raised if VLC is too old.""" + + +class UnexpectedInstanceParameterError(DakaraError): + """Error raised when passing incorrect parameters to the VLC instance.""" diff --git a/tests/integration/test_media_player_vlc.py b/tests/integration/test_media_player_vlc.py index 83d7269..ff54b45 100644 --- a/tests/integration/test_media_player_vlc.py +++ b/tests/integration/test_media_player_vlc.py @@ -4,7 +4,7 @@ from tempfile import TemporaryDirectory from threading import Event from unittest import skipIf, skipUnless -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch try: import vlc @@ -16,7 +16,11 @@ from func_timeout import func_set_timeout from dakara_player.media_player.base import IDLE_BG_NAME, TRANSITION_BG_NAME -from dakara_player.media_player.vlc import METADATA_KEYS_COUNT, MediaPlayerVlc +from dakara_player.media_player.vlc import ( + METADATA_KEYS_COUNT, + MediaPlayerVlc, + UnexpectedInstanceParameterError, +) from dakara_player.mrl import mrl_to_path from tests.integration.base import TestCasePollerKara @@ -113,6 +117,18 @@ def get_instance(self, config=None, check_error=True): # assert no errors to fail test if any self.assertFalse(vlc_player.stop.is_set()) + @patch( + "dakara_player.media_player.vlc.vlc.Instance", return_value=None, autospec=True + ) + @patch.object(MediaPlayerVlc, "is_available", return_value=True, autospec=True) + def test_init_invalid_instance_parameters( + self, mocked_is_available, mocked_instance + ): + """Test to pass invalid instance parameters.""" + with self.assertRaises(UnexpectedInstanceParameterError): + with self.get_instance(): + pass + def test_metadata_keys_count(self): """Test the number of metadata keys.""" self.assertNotEqual(METADATA_KEYS_COUNT, 0) From 111338618ce6757e2d2d4e220ba0f2019f7f342d Mon Sep 17 00:00:00 2001 From: Neraste Date: Tue, 4 Aug 2026 00:32:31 +0200 Subject: [PATCH 2/4] Better implementation --- src/dakara_player/media_player/vlc.py | 27 ++++++++++++++++++---- tests/integration/test_media_player_vlc.py | 15 +----------- tests/unit/test_media_player_vlc.py | 13 +++++++++++ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/dakara_player/media_player/vlc.py b/src/dakara_player/media_player/vlc.py index 7a0dbed..cb9507e 100644 --- a/src/dakara_player/media_player/vlc.py +++ b/src/dakara_player/media_player/vlc.py @@ -123,11 +123,7 @@ def init_player(self, config, tempdir): ) # VLC objects - instance = vlc.Instance(config_vlc.get("instance_parameters") or []) - if instance is None: - raise UnexpectedInstanceParameterError("Unexpected instance parameter") - - self.instance = instance + self.instance = get_instance(config_vlc.get("instance_parameters")) player = self.instance.media_player_new() self.player = player @@ -792,6 +788,27 @@ def get_metadata(media): raise ValueError("This media has no set metadata") +def get_instance(instance_parameters=None): + """Get a VLC instance with parameters. + + Args: + instance_parameters (list of str): List of parameters. Must be in the + form "--option=value". + + Returns: + vlc.Instance: New instance. + + Raises: + UnexpectedInstanceParameterError: If unexpected parameters are + passed. + """ + instance = vlc.Instance(instance_parameters or []) + if instance is None: + raise UnexpectedInstanceParameterError("Unexpected instance parameter") + + return instance + + class Media: """Media object.""" diff --git a/tests/integration/test_media_player_vlc.py b/tests/integration/test_media_player_vlc.py index ff54b45..86c73cb 100644 --- a/tests/integration/test_media_player_vlc.py +++ b/tests/integration/test_media_player_vlc.py @@ -4,7 +4,7 @@ from tempfile import TemporaryDirectory from threading import Event from unittest import skipIf, skipUnless -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock try: import vlc @@ -19,7 +19,6 @@ from dakara_player.media_player.vlc import ( METADATA_KEYS_COUNT, MediaPlayerVlc, - UnexpectedInstanceParameterError, ) from dakara_player.mrl import mrl_to_path from tests.integration.base import TestCasePollerKara @@ -117,18 +116,6 @@ def get_instance(self, config=None, check_error=True): # assert no errors to fail test if any self.assertFalse(vlc_player.stop.is_set()) - @patch( - "dakara_player.media_player.vlc.vlc.Instance", return_value=None, autospec=True - ) - @patch.object(MediaPlayerVlc, "is_available", return_value=True, autospec=True) - def test_init_invalid_instance_parameters( - self, mocked_is_available, mocked_instance - ): - """Test to pass invalid instance parameters.""" - with self.assertRaises(UnexpectedInstanceParameterError): - with self.get_instance(): - pass - def test_metadata_keys_count(self): """Test the number of metadata keys.""" self.assertNotEqual(METADATA_KEYS_COUNT, 0) diff --git a/tests/unit/test_media_player_vlc.py b/tests/unit/test_media_player_vlc.py index 527d973..647539a 100644 --- a/tests/unit/test_media_player_vlc.py +++ b/tests/unit/test_media_player_vlc.py @@ -24,7 +24,9 @@ ) from dakara_player.media_player.vlc import ( MediaPlayerVlc, + UnexpectedInstanceParameterError, VlcTooOldError, + get_instance, get_metadata, set_metadata, ) @@ -1422,3 +1424,14 @@ def test_not_in_list_default_return(self, mocked_is_playing_this): with self.get_instance() as (player, _, _): self.assertEqual(function_decorated(player), 42) + + +class GetInstanceTestCase(TestCase): + + @patch( + "dakara_player.media_player.vlc.vlc.Instance", return_value=None, autospec=True + ) + def test_unexpected_instance_parameters(self, mocked_instance): + """Test to pass unexpected instance parameters.""" + with self.assertRaises(UnexpectedInstanceParameterError): + get_instance() From 00fac0a8f89bdc265342e4e9f420b019998d45b1 Mon Sep 17 00:00:00 2001 From: Neraste Date: Tue, 4 Aug 2026 00:56:47 +0200 Subject: [PATCH 3/4] Simplify test mock --- tests/unit/test_media_player_vlc.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_media_player_vlc.py b/tests/unit/test_media_player_vlc.py index 647539a..ab2e97b 100644 --- a/tests/unit/test_media_player_vlc.py +++ b/tests/unit/test_media_player_vlc.py @@ -1428,10 +1428,9 @@ def test_not_in_list_default_return(self, mocked_is_playing_this): class GetInstanceTestCase(TestCase): - @patch( - "dakara_player.media_player.vlc.vlc.Instance", return_value=None, autospec=True - ) - def test_unexpected_instance_parameters(self, mocked_instance): + @patch("dakara_player.media_player.vlc.vlc", autospec=True) + def test_unexpected_instance_parameters(self, mocked_vlc): """Test to pass unexpected instance parameters.""" + mocked_vlc.Instance.return_value = None with self.assertRaises(UnexpectedInstanceParameterError): get_instance() From 28eb14992bbdf729d94b8bdf15626ebafa39db98 Mon Sep 17 00:00:00 2001 From: Neraste Date: Sat, 8 Aug 2026 18:14:11 +0200 Subject: [PATCH 4/4] Separate if parameters are given to the instance or not --- src/dakara_player/media_player/vlc.py | 19 +++++++++++++++++-- tests/unit/test_media_player_vlc.py | 23 ++++++++++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/dakara_player/media_player/vlc.py b/src/dakara_player/media_player/vlc.py index cb9507e..8f7de0d 100644 --- a/src/dakara_player/media_player/vlc.py +++ b/src/dakara_player/media_player/vlc.py @@ -799,12 +799,23 @@ def get_instance(instance_parameters=None): vlc.Instance: New instance. Raises: + UnavailableInstanceError: If an instance cannot be obtained without + parameters. UnexpectedInstanceParameterError: If unexpected parameters are passed. """ - instance = vlc.Instance(instance_parameters or []) + # if no parameters are passed, the instance should never be None + if not instance_parameters: + instance = vlc.Instance() + if instance is None: + raise UnavailableInstanceError("Unable to get a VLC instance") + + return instance + + # if parameters are passed, an unexpected parameter makes the instance None + instance = vlc.Instance(instance_parameters) if instance is None: - raise UnexpectedInstanceParameterError("Unexpected instance parameter") + raise UnexpectedInstanceParameterError("Unexpected VLC instance parameter") return instance @@ -831,3 +842,7 @@ class VlcTooOldError(DakaraError): class UnexpectedInstanceParameterError(DakaraError): """Error raised when passing incorrect parameters to the VLC instance.""" + + +class UnavailableInstanceError(DakaraError): + """Error raised when a VLC instance cannot be obtained.""" diff --git a/tests/unit/test_media_player_vlc.py b/tests/unit/test_media_player_vlc.py index ab2e97b..3edc5be 100644 --- a/tests/unit/test_media_player_vlc.py +++ b/tests/unit/test_media_player_vlc.py @@ -24,6 +24,7 @@ ) from dakara_player.media_player.vlc import ( MediaPlayerVlc, + UnavailableInstanceError, UnexpectedInstanceParameterError, VlcTooOldError, get_instance, @@ -1426,11 +1427,27 @@ def test_not_in_list_default_return(self, mocked_is_playing_this): self.assertEqual(function_decorated(player), 42) +@patch("dakara_player.media_player.vlc.vlc", autospec=True) class GetInstanceTestCase(TestCase): - @patch("dakara_player.media_player.vlc.vlc", autospec=True) - def test_unexpected_instance_parameters(self, mocked_vlc): - """Test to pass unexpected instance parameters.""" + def test_parameters_unexpected(self, mocked_vlc): + """Test to pass unexpected parameters.""" mocked_vlc.Instance.return_value = None with self.assertRaises(UnexpectedInstanceParameterError): + get_instance(["parameter"]) + + def test_parameters(self, mocked_vlc): + """Test to pass parameters.""" + self.assertIs(mocked_vlc.Instance.return_value, get_instance(["parameter"])) + mocked_vlc.Instance.assert_called_with(["parameter"]) + + def test_no_parameters_unavailable(self, mocked_vlc): + """Test unavailable instance.""" + mocked_vlc.Instance.return_value = None + with self.assertRaises(UnavailableInstanceError): get_instance() + + def test_no_parameters(self, mocked_vlc): + """Test instance.""" + self.assertIs(mocked_vlc.Instance.return_value, get_instance()) + mocked_vlc.Instance.assert_called_with()