Merge pull request #140 from DakaraProject/refactor/instrumental-tracks

Improve management of instrumental tracks

Author: Neraste

src/dakara_player/media_player/mpv.py

@@ -40,6 +40,8 @@
 
 PLAYER_IS_AVAILABLE_ATTEMPTS = 5
 
+USE_PATH_AUDIO = -1
+
 
 # monkey patch mpv to silent socket close failures on windows
 if mpv is not None:
@@ -395,18 +397,17 @@ def play(self, what):
 
         if what == "song":
             # manage instrumental track/file
-            path_audio = self.playlist_entry_data["song"].path_audio
-            if path_audio:
-                if path_audio == "self":
-                    # mpv use different index for each track, so we can safely request
-                    # the second audio track
-                    self.player.audio = 2
-                    logger.debug("Requesting to play audio track 2")
-
-                else:
+            track_id_audio = self.playlist_entry_data["song"].track_id_audio
+            if track_id_audio is not None:
+                if track_id_audio == USE_PATH_AUDIO:
+                    path_audio = self.playlist_entry_data["song"].path_audio
                     self.player.audio_files = [str(path_audio)]
                     logger.debug("Requesting to play audio file %s", path_audio)
 
+                else:
+                    self.player.audio = track_id_audio
+                    logger.debug("Requesting to play audio track %i", track_id_audio)
+
             # if the subtitle file cannot be discovered, do not request it
             if self.playlist_entry_data["song"].path_subtitle:
                 self.player.sub_files = [
@@ -581,6 +582,7 @@ def manage_instrumental(self, playlist_entry, file_path):
         audio_path = self.get_instrumental_file(file_path)
 
         if audio_path:
+            self.playlist_entry_data["song"].track_id_audio = USE_PATH_AUDIO
             self.playlist_entry_data["song"].path_audio = audio_path
             logger.info(
                 "Requesting to play instrumental file '%s' for '%s'",
@@ -590,9 +592,10 @@ def manage_instrumental(self, playlist_entry, file_path):
 
             return
 
-        # otherwise mark to look for instrumental track in internal tracks when
-        # starting to read the media
-        self.playlist_entry_data["song"].path_audio = "self"
+        # otherwise mark to use the 2nd track when starting to read the media
+        # mpv use different index for each track, so we can safely request the
+        # second audio track
+        self.playlist_entry_data["song"].track_id_audio = 2
         logger.info("Requesting to play instrumental track of '%s'", file_path)
 
     def clear_playlist_entry_player(self):
@@ -723,7 +726,7 @@ def handle_start_file(self, event):
 
         raise InvalidStateError("Start file on an undeterminated state")
 
-    def get_audio_tracks_id(self):
+    def get_track_id_audio_list(self):
         """Get ID of audio tracks for the current media.
 
         Returns:
@@ -1034,10 +1037,13 @@ def __init__(self, path=None):
 class MediaSong(Media):
     """Song class."""
 
-    def __init__(self, *args, path_subtitle=None, path_audio=None, **kwargs):
+    def __init__(
+        self, *args, path_subtitle=None, path_audio=None, track_id_audio=None, **kwargs
+    ):
         super().__init__(*args, **kwargs)
         self.path_subtitle = path_subtitle
         self.path_audio = path_audio
+        self.track_id_audio = track_id_audio
 
 
 class MpvTooOldError(DakaraError):

src/dakara_player/media_player/vlc.py

@@ -500,18 +500,18 @@ def manage_instrumental(self, playlist_entry, file_path):
                 )
                 return
 
-            self.playlist_entry_data["song"].audio_track_id = number_tracks
+            self.playlist_entry_data["song"].track_id_audio = number_tracks
             return
 
-        # get audio tracks
-        audio_tracks_id = self.get_audio_tracks_id(
+        # get audio track ids
+        track_id_audio_list = self.get_track_id_audio_list(
             self.playlist_entry_data["song"].media
         )
 
         # if more than 1 audio track is present, register to play the 2nd one
-        if len(audio_tracks_id) > 1:
+        if len(track_id_audio_list) > 1:
             logger.info("Requesting to play instrumental track of '%s'", file_path)
-            self.playlist_entry_data["song"].audio_track_id = audio_tracks_id[1]
+            self.playlist_entry_data["song"].track_id_audio = track_id_audio_list[1]
             return
 
         # otherwise, fallback to register to play the first track and log it
@@ -539,7 +539,7 @@ def get_number_tracks(media):
         return len(list(media.tracks_get()))
 
     @staticmethod
-    def get_audio_tracks_id(media):
+    def get_track_id_audio_list(media):
         """Get ID of audio tracks of the media.
 
         Args:
@@ -681,10 +681,10 @@ def handle_playing(self, event):
             self.callbacks["started_song"](self.playlist_entry["id"])
 
             # set instrumental track if necessary
-            audio_track_id = self.playlist_entry_data["song"].audio_track_id
-            if audio_track_id is not None:
-                logger.debug("Requesting to play audio track %i", audio_track_id)
-                self.player.audio_set_track(audio_track_id)
+            track_id_audio = self.playlist_entry_data["song"].track_id_audio
+            if track_id_audio is not None:
+                logger.debug("Requesting to play audio track %i", track_id_audio)
+                self.player.audio_set_track(track_id_audio)
 
             self.playlist_entry_data["song"].started = True
             logger.info(
@@ -803,9 +803,9 @@ def __init__(self, media=None):
 class MediaSong(Media):
     """Song object."""
 
-    def __init__(self, *args, audio_track_id=None, **kwargs):
+    def __init__(self, *args, track_id_audio=None, **kwargs):
         super().__init__(*args, **kwargs)
-        self.audio_track_id = audio_track_id
+        self.track_id_audio = track_id_audio
 
 
 class VlcTooOldError(DakaraError):

tests/integration/test_media_player_mpv.py

@@ -245,7 +245,7 @@ def test_play_playlist_entry_instrumental_track(self):
             self.wait_is_playing(mpv_player, "song")
 
             # check the current media has 2 audio tracks
-            self.assertListEqual(mpv_player.get_audio_tracks_id(), [1, 2])
+            self.assertListEqual(mpv_player.get_track_id_audio_list(), [1, 2])
 
             # check media exists
             self.assertIsNotNone(mpv_player.player.path)

tests/unit/test_media_player_vlc.py

@@ -475,22 +475,22 @@ def test_set_playlist_entry(
             mocked_play.assert_called_with("transition")
             mocked_manage_instrumental.assert_not_called()
 
-    @patch.object(MediaPlayerVlc, "get_audio_tracks_id")
+    @patch.object(MediaPlayerVlc, "get_track_id_audio_list")
     @patch.object(MediaPlayerVlc, "get_number_tracks")
     @patch.object(MediaPlayerVlc, "get_instrumental_file")
     def test_manage_instrumental_file(
         self,
         mocked_get_instrumental_file,
         mocked_get_number_tracks,
-        mocked_get_audio_tracks_id,
+        mocked_get_track_id_audio_list,
     ):
         """Test to add instrumental file."""
         with self.get_instance() as (vlc_player, (mocked_instance, _, _), _):
             video_path = get_temp_dir() / "video"
             audio_path = get_temp_dir() / "audio"
 
             # pre assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
             self.assertIsNotNone(vlc_player.kara_folder_path)
 
             # set playlist entry to request instrumental
@@ -507,7 +507,7 @@ def test_manage_instrumental_file(
                 vlc_player.manage_instrumental(self.playlist_entry, video_path)
 
             # post assertions
-            self.assertEqual(vlc_player.playlist_entry_data["song"].audio_track_id, 2)
+            self.assertEqual(vlc_player.playlist_entry_data["song"].track_id_audio, 2)
 
             # assert the effects on logs
             self.assertListEqual(
@@ -519,7 +519,7 @@ def test_manage_instrumental_file(
             )
 
             # assert the call
-            mocked_get_audio_tracks_id.assert_not_called()
+            mocked_get_track_id_audio_list.assert_not_called()
 
     @patch.object(MediaPlayerVlc, "get_number_tracks")
     @patch.object(MediaPlayerVlc, "get_instrumental_file")
@@ -534,7 +534,7 @@ def test_manage_instrumental_file_error_slaves_add(
             audio_path = get_temp_dir() / "audio"
 
             # pre assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
             self.assertIsNotNone(vlc_player.kara_folder_path)
 
             # set playlist entry to request instrumental
@@ -554,7 +554,7 @@ def test_manage_instrumental_file_error_slaves_add(
                 vlc_player.manage_instrumental(self.playlist_entry, video_path)
 
             # post assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
 
             # assert the effects on logs
             self.assertListEqual(
@@ -567,14 +567,14 @@ def test_manage_instrumental_file_error_slaves_add(
                 ],
             )
 
-    @patch.object(MediaPlayerVlc, "get_audio_tracks_id")
+    @patch.object(MediaPlayerVlc, "get_track_id_audio_list")
     @patch.object(MediaPlayerVlc, "get_number_tracks")
     @patch.object(MediaPlayerVlc, "get_instrumental_file")
     def test_manage_instrumental_track(
         self,
         mocked_get_instrumental_file,
         mocked_get_number_tracks,
-        mocked_get_audio_tracks_id,
+        mocked_get_track_id_audio_list,
     ):
         """Test add instrumental track."""
         with self.get_instance() as (
@@ -589,15 +589,15 @@ def test_manage_instrumental_track(
             video_path = get_temp_dir() / "video"
 
             # pre assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
             self.assertIsNotNone(vlc_player.kara_folder_path)
 
             # set playlist entry to request instrumental
             self.playlist_entry["use_instrumental"] = True
 
             # mocks
             mocked_get_instrumental_file.return_value = None
-            mocked_get_audio_tracks_id.return_value = [0, 99, 42]
+            mocked_get_track_id_audio_list.return_value = [0, 99, 42]
             mocked_media_song = mocked_instance.media_new_path.return_value
             vlc_player.playlist_entry_data["song"].media = mocked_media_song
 
@@ -606,7 +606,7 @@ def test_manage_instrumental_track(
                 vlc_player.manage_instrumental(self.playlist_entry, video_path)
 
             # post assertions
-            self.assertEqual(vlc_player.playlist_entry_data["song"].audio_track_id, 99)
+            self.assertEqual(vlc_player.playlist_entry_data["song"].track_id_audio, 99)
 
             # assert the effects on logs
             self.assertListEqual(
@@ -620,24 +620,24 @@ def test_manage_instrumental_track(
             # assert the call
             mocked_get_number_tracks.assert_not_called()
 
-    @patch.object(MediaPlayerVlc, "get_audio_tracks_id")
+    @patch.object(MediaPlayerVlc, "get_track_id_audio_list")
     @patch.object(MediaPlayerVlc, "get_instrumental_file")
     def test_manage_instrumental_no_instrumental_found(
-        self, mocked_get_instrumental_file, mocked_get_audio_tracks_id
+        self, mocked_get_instrumental_file, mocked_get_track_id_audio_list
     ):
         """Test to cannot find instrumental."""
         with self.get_instance() as (vlc_player, (mocked_instance, _, _), _):
             video_path = get_temp_dir() / "video"
 
             # pre assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
 
             # set playlist entry to request instrumental
             self.playlist_entry["use_instrumental"] = True
 
             # mocks
             mocked_get_instrumental_file.return_value = None
-            mocked_get_audio_tracks_id.return_value = [99]
+            mocked_get_track_id_audio_list.return_value = [99]
 
             # make slaves_add method unavailable
             mocked_media_song = mocked_instance.return_value.media_new_path.return_value
@@ -648,7 +648,7 @@ def test_manage_instrumental_no_instrumental_found(
                 vlc_player.manage_instrumental(self.playlist_entry, video_path)
 
             # post assertions
-            self.assertIsNone(vlc_player.playlist_entry_data["song"].audio_track_id)
+            self.assertIsNone(vlc_player.playlist_entry_data["song"].track_id_audio)
 
             # assert the effects on logs
             self.assertListEqual(
@@ -1009,7 +1009,7 @@ def test_handle_playing_media_starts_track_id(self, mocked_is_playing_this):
         with self.get_instance() as (vlc_player, (mocked_instance, _, _), _):
             mocked_player = mocked_instance.media_player_new.return_value
             self.set_playlist_entry(vlc_player)
-            vlc_player.playlist_entry_data["song"].audio_track_id = 99
+            vlc_player.playlist_entry_data["song"].track_id_audio = 99
 
             # mock the call
             vlc_player.set_callback("started_song", MagicMock())