From 1ecb6d6e90ca9886d56c41a168e2665148ef2867 Mon Sep 17 00:00:00 2001 From: Soaku Date: Fri, 24 Jul 2020 09:42:22 +0200 Subject: [PATCH 01/11] Fix gapless playback --- mopidy_spotify/playback.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy_spotify/playback.py b/mopidy_spotify/playback.py index d86d6da2..b9514c8b 100644 --- a/mopidy_spotify/playback.py +++ b/mopidy_spotify/playback.py @@ -208,6 +208,9 @@ def end_of_track_callback(session, end_of_track_event, audio_actor): end_of_track_event.set() audio_actor.emit_data(None) + # unload the player to stop receiving data + session.player.unload() + class BufferTimestamp: """Wrapper around an int to serialize access by multiple threads. From ecafd9ab459ae6abbc3ce2a78b0a2eae41a08523 Mon Sep 17 00:00:00 2001 From: Soaku Date: Sat, 25 Jul 2020 12:43:53 +0200 Subject: [PATCH 02/11] Add buffer holding --- mopidy_spotify/playback.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/mopidy_spotify/playback.py b/mopidy_spotify/playback.py index b9514c8b..5f5963c4 100644 --- a/mopidy_spotify/playback.py +++ b/mopidy_spotify/playback.py @@ -15,6 +15,11 @@ # Extra log level with lower importance than DEBUG=10 for noisy debug logging TRACE_LOG_LEVEL = 5 +# Last audio data sent to the buffer, currently on hold. +# This data is held because libspotify sends a single empty buffer before ending the track. It is discarded +# the moment a new track starts so a smooth transition between songs can be made. +_held_buffer = None + class SpotifyPlaybackProvider(backend.PlaybackProvider): def __init__(self, *args, **kwargs): @@ -50,6 +55,8 @@ def _connect_events(self): def change_track(self, track): self._connect_events() + global _held_buffer + if track.uri is None: return False @@ -79,6 +86,9 @@ def change_track(self, track): self.backend._session.player.load(sp_track) self.backend._session.player.play() + # Discard held buffer + _held_buffer = None + future = self.audio.set_appsrc( GST_CAPS, need_data=need_data_callback_bound, @@ -162,6 +172,8 @@ def music_delivery_callback( # This is called from an internal libspotify thread. # Ideally, nothing here should block. + global _held_buffer + if seeking_event.is_set(): # A seek has happened, but libspotify hasn't confirmed yet, so # we're dropping all audio data from libspotify. @@ -187,8 +199,15 @@ def music_delivery_callback( bytes(frames), timestamp=buffer_timestamp.get(), duration=duration ) - # We must block here to know if the buffer was consumed successfully. - consumed = audio_actor.emit_data(buffer_).get() + # If there is any held buffer, send it + if _held_buffer: + consumed = audio_actor.emit_data(_held_buffer).get() + else: + # No buffer, we assume successful consumption + consumed = True + + # Hold the buffer for a while, so we can filter out the single empty buffer after the track + _held_buffer = buffer_ if consumed: buffer_timestamp.increase(duration) @@ -208,7 +227,7 @@ def end_of_track_callback(session, end_of_track_event, audio_actor): end_of_track_event.set() audio_actor.emit_data(None) - # unload the player to stop receiving data + # Stop the track to prevent receiving empty audio data session.player.unload() From 69e294afc88108e4fa7b7e6c3cf5e5836526e964 Mon Sep 17 00:00:00 2001 From: Soaku Date: Tue, 25 Aug 2020 16:45:25 +0200 Subject: [PATCH 03/11] Move buffer clearing to avoid race conditions --- mopidy_spotify/playback.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy_spotify/playback.py b/mopidy_spotify/playback.py index 5f5963c4..124e7472 100644 --- a/mopidy_spotify/playback.py +++ b/mopidy_spotify/playback.py @@ -80,15 +80,15 @@ def change_track(self, track): self._first_seek = True self._end_of_track_event.clear() + # Discard held buffer + _held_buffer = None + try: sp_track = self.backend._session.get_track(track.uri) sp_track.load(self._timeout) self.backend._session.player.load(sp_track) self.backend._session.player.play() - # Discard held buffer - _held_buffer = None - future = self.audio.set_appsrc( GST_CAPS, need_data=need_data_callback_bound, From 7be0d1be71f3d1eb1ca97ef31d261c939d511e49 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Tue, 29 Sep 2020 10:10:21 +0100 Subject: [PATCH 04/11] Avoid using global variable for held buffer. This fixes the tests. Also added testing of the held buffer. --- mopidy_spotify/playback.py | 34 +++++++++++-------- tests/test_playback.py | 68 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/mopidy_spotify/playback.py b/mopidy_spotify/playback.py index 124e7472..70e2071c 100644 --- a/mopidy_spotify/playback.py +++ b/mopidy_spotify/playback.py @@ -15,10 +15,10 @@ # Extra log level with lower importance than DEBUG=10 for noisy debug logging TRACE_LOG_LEVEL = 5 -# Last audio data sent to the buffer, currently on hold. -# This data is held because libspotify sends a single empty buffer before ending the track. It is discarded -# the moment a new track starts so a smooth transition between songs can be made. -_held_buffer = None +# Last audio data sent to the buffer, currently on hold. This data is held +# because libspotify sends a single empty buffer before ending the track. It is +# discarded the moment a new track starts so a smooth transition between songs +# can be made. class SpotifyPlaybackProvider(backend.PlaybackProvider): @@ -33,6 +33,7 @@ def __init__(self, *args, **kwargs): self._push_audio_data_event.set() self._end_of_track_event = threading.Event() self._events_connected = False + self._held_buffer = BufferData() def _connect_events(self): if not self._events_connected: @@ -44,6 +45,7 @@ def _connect_events(self): self._seeking_event, self._push_audio_data_event, self._buffer_timestamp, + self._held_buffer, ) self.backend._session.on( spotify.SessionEvent.END_OF_TRACK, @@ -55,8 +57,6 @@ def _connect_events(self): def change_track(self, track): self._connect_events() - global _held_buffer - if track.uri is None: return False @@ -81,7 +81,7 @@ def change_track(self, track): self._end_of_track_event.clear() # Discard held buffer - _held_buffer = None + self._held_buffer.data = None try: sp_track = self.backend._session.get_track(track.uri) @@ -168,12 +168,11 @@ def music_delivery_callback( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ): # This is called from an internal libspotify thread. # Ideally, nothing here should block. - global _held_buffer - if seeking_event.is_set(): # A seek has happened, but libspotify hasn't confirmed yet, so # we're dropping all audio data from libspotify. @@ -200,14 +199,16 @@ def music_delivery_callback( ) # If there is any held buffer, send it - if _held_buffer: - consumed = audio_actor.emit_data(_held_buffer).get() + # change_track() in other thread could clobber before we emit_data. OK? + if held_buffer.data is not None: + consumed = audio_actor.emit_data(held_buffer.data).get() else: # No buffer, we assume successful consumption consumed = True - # Hold the buffer for a while, so we can filter out the single empty buffer after the track - _held_buffer = buffer_ + # Hold the buffer for a while, so we can filter out the single empty buffer + # after the track + held_buffer.data = buffer_ if consumed: buffer_timestamp.increase(duration) @@ -253,3 +254,10 @@ def set(self, value): def increase(self, value): with self._lock: self._value += value + + +class BufferData: + """Wrapper around an Gst.Buffer to pass around.""" + + def __init__(self, buf=None): + self.data = buf diff --git a/tests/test_playback.py b/tests/test_playback.py index 3782daea..812d5396 100644 --- a/tests/test_playback.py +++ b/tests/test_playback.py @@ -64,6 +64,7 @@ def test_connect_events_adds_music_delivery_handler_to_session( playback_provider._seeking_event, playback_provider._push_audio_data_event, playback_provider._buffer_timestamp, + playback_provider._held_buffer, ) in session_mock.on.call_args_list ) @@ -130,6 +131,22 @@ def test_change_track_sets_up_appsrc(audio_mock, provider): audio_mock.set_metadata.assert_called_once_with(track) +def test_change_track_clears_state(audio_mock, provider): + track = models.Track(uri="spotfy:track:test") + + provider._first_seek = False + provider._buffer_timestamp.set(99) + provider._end_of_track_event.set() + provider._held_buffer.data = mock.sentinel.gst_buffer + + assert provider.change_track(track) is True + + assert provider._first_seek + assert provider._buffer_timestamp.get() == 0 + assert not provider._end_of_track_event.is_set() + assert provider._held_buffer.data is None + + def test_resume_starts_spotify_playback(session_mock, provider): provider.resume() @@ -208,6 +225,7 @@ def test_music_delivery_rejects_data_when_seeking(session_mock, audio_mock): push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) assert seeking_event.is_set() result = playback.music_delivery_callback( @@ -219,6 +237,7 @@ def test_music_delivery_rejects_data_when_seeking(session_mock, audio_mock): seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert seeking_event.is_set() @@ -238,6 +257,7 @@ def test_music_delivery_when_seeking_accepts_data_after_empty_delivery( push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) assert seeking_event.is_set() result = playback.music_delivery_callback( @@ -249,6 +269,7 @@ def test_music_delivery_when_seeking_accepts_data_after_empty_delivery( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert not seeking_event.is_set() @@ -266,6 +287,7 @@ def test_music_delivery_rejects_data_depending_on_push_audio_data_event( seeking_event = threading.Event() push_audio_data_event = threading.Event() buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) assert not push_audio_data_event.is_set() result = playback.music_delivery_callback( @@ -277,6 +299,7 @@ def test_music_delivery_rejects_data_depending_on_push_audio_data_event( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert audio_mock.emit_data.call_count == 0 @@ -294,6 +317,7 @@ def test_music_delivery_shortcuts_if_no_data_in_frames( push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) result = playback.music_delivery_callback( session_mock, @@ -304,6 +328,7 @@ def test_music_delivery_shortcuts_if_no_data_in_frames( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert result == 0 @@ -320,6 +345,7 @@ def test_music_delivery_rejects_unknown_audio_formats(session_mock, audio_mock): push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) with pytest.raises(AssertionError) as excinfo: playback.music_delivery_callback( @@ -331,12 +357,13 @@ def test_music_delivery_rejects_unknown_audio_formats(session_mock, audio_mock): seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert "Expects 16-bit signed integer samples" in str(excinfo.value) -def test_music_delivery_creates_gstreamer_buffer_and_gives_it_to_audio( +def test_music_delivery_creates_gstreamer_buffer_and_holds_it( session_mock, audio_mock, audio_lib_mock ): @@ -351,6 +378,7 @@ def test_music_delivery_creates_gstreamer_buffer_and_gives_it_to_audio( push_audio_data_event.set() buffer_timestamp = mock.Mock() buffer_timestamp.get.return_value = mock.sentinel.timestamp + held_buffer = playback.BufferData(None) result = playback.music_delivery_callback( session_mock, @@ -361,6 +389,7 @@ def test_music_delivery_creates_gstreamer_buffer_and_gives_it_to_audio( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) audio_lib_mock.calculate_duration.assert_called_once_with(1, 44100) @@ -370,14 +399,46 @@ def test_music_delivery_creates_gstreamer_buffer_and_gives_it_to_audio( duration=mock.sentinel.duration, ) buffer_timestamp.increase.assert_called_once_with(mock.sentinel.duration) - audio_mock.emit_data.assert_called_once_with(mock.sentinel.gst_buffer) + audio_mock.emit_data.assert_not_called() + assert result == num_frames + assert held_buffer.data == mock.sentinel.gst_buffer + + +def test_music_delivery_gives_held_buffer_to_audio_and_holds_created( + session_mock, audio_mock, audio_lib_mock +): + audio_lib_mock.create_buffer.return_value = mock.sentinel.gst_buffer2 + + audio_format = mock.Mock(channels=2, sample_rate=44100, sample_type=0) + frames = b"\x00\x00" + num_frames = 1 + seeking_event = threading.Event() + push_audio_data_event = threading.Event() + push_audio_data_event.set() + buffer_timestamp = mock.Mock() + held_buffer = playback.BufferData(mock.sentinel.gst_buffer1) + + result = playback.music_delivery_callback( + session_mock, + audio_format, + frames, + num_frames, + audio_mock, + seeking_event, + push_audio_data_event, + buffer_timestamp, + held_buffer, + ) assert result == num_frames + audio_mock.emit_data.assert_called_once_with(mock.sentinel.gst_buffer1) + assert held_buffer.data == mock.sentinel.gst_buffer2 def test_music_delivery_consumes_zero_frames_if_audio_fails( session_mock, audio_mock, audio_lib_mock ): + audio_lib_mock.create_buffer.return_value = mock.sentinel.gst_buffer2 audio_mock.emit_data.return_value.get.return_value = False audio_format = mock.Mock(channels=2, sample_rate=44100, sample_type=0) @@ -388,6 +449,7 @@ def test_music_delivery_consumes_zero_frames_if_audio_fails( push_audio_data_event.set() buffer_timestamp = mock.Mock() buffer_timestamp.get.return_value = mock.sentinel.timestamp + held_buffer = playback.BufferData(mock.sentinel.gst_buffer) result = playback.music_delivery_callback( session_mock, @@ -398,10 +460,12 @@ def test_music_delivery_consumes_zero_frames_if_audio_fails( seeking_event, push_audio_data_event, buffer_timestamp, + held_buffer, ) assert buffer_timestamp.increase.call_count == 0 assert result == 0 + assert held_buffer.data == mock.sentinel.gst_buffer2 # SPONG def test_end_of_track_callback(session_mock, audio_mock): From 0ac120d090811e8da113a0115ee1d3773bf3084b Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Tue, 24 Nov 2020 01:01:16 +0000 Subject: [PATCH 05/11] Keep held buffer and reject new buffer when audio back-pressures. --- mopidy_spotify/playback.py | 48 +++++++++++++++++--------------------- tests/test_playback.py | 29 ++++++++++++----------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/mopidy_spotify/playback.py b/mopidy_spotify/playback.py index 70e2071c..8c024d36 100644 --- a/mopidy_spotify/playback.py +++ b/mopidy_spotify/playback.py @@ -1,4 +1,5 @@ import functools +import collections import logging import threading @@ -15,11 +16,6 @@ # Extra log level with lower importance than DEBUG=10 for noisy debug logging TRACE_LOG_LEVEL = 5 -# Last audio data sent to the buffer, currently on hold. This data is held -# because libspotify sends a single empty buffer before ending the track. It is -# discarded the moment a new track starts so a smooth transition between songs -# can be made. - class SpotifyPlaybackProvider(backend.PlaybackProvider): def __init__(self, *args, **kwargs): @@ -33,7 +29,11 @@ def __init__(self, *args, **kwargs): self._push_audio_data_event.set() self._end_of_track_event = threading.Event() self._events_connected = False - self._held_buffer = BufferData() + # libspotify sends a single empty buffer at the end of each track which + # must be discarded to ensure a gapless track transition. We delay using + # each buffer until we receieve the next one, up until we change track + # and clear everything, therefore dropping the unwanted last buffer. + self._held_buffers = collections.deque() def _connect_events(self): if not self._events_connected: @@ -45,7 +45,7 @@ def _connect_events(self): self._seeking_event, self._push_audio_data_event, self._buffer_timestamp, - self._held_buffer, + self._held_buffers, ) self.backend._session.on( spotify.SessionEvent.END_OF_TRACK, @@ -80,8 +80,8 @@ def change_track(self, track): self._first_seek = True self._end_of_track_event.clear() - # Discard held buffer - self._held_buffer.data = None + # Discard held buffers + self._held_buffers.clear() try: sp_track = self.backend._session.get_track(track.uri) @@ -168,7 +168,7 @@ def music_delivery_callback( seeking_event, push_audio_data_event, buffer_timestamp, - held_buffer, + held_buffers, ): # This is called from an internal libspotify thread. # Ideally, nothing here should block. @@ -198,22 +198,25 @@ def music_delivery_callback( bytes(frames), timestamp=buffer_timestamp.get(), duration=duration ) - # If there is any held buffer, send it - # change_track() in other thread could clobber before we emit_data. OK? - if held_buffer.data is not None: - consumed = audio_actor.emit_data(held_buffer.data).get() + # Try to consume any held buffers. + if held_buffers: + while held_buffers: + buf = held_buffers.popleft() + consumed = audio_actor.emit_data(buf).get() + if not consumed: + held_buffers.appendleft(buf) + break else: - # No buffer, we assume successful consumption + # No held buffer, don't apply back-pressure consumed = True - # Hold the buffer for a while, so we can filter out the single empty buffer - # after the track - held_buffer.data = buffer_ - if consumed: + # Consumed all held buffers so take the new one libspotify delivered us. + held_buffers.append(buffer_) buffer_timestamp.increase(duration) return num_frames else: + # Pass back-pressure on to libspotify, next buffer will be redelivered. return 0 @@ -254,10 +257,3 @@ def set(self, value): def increase(self, value): with self._lock: self._value += value - - -class BufferData: - """Wrapper around an Gst.Buffer to pass around.""" - - def __init__(self, buf=None): - self.data = buf diff --git a/tests/test_playback.py b/tests/test_playback.py index 812d5396..d732c62f 100644 --- a/tests/test_playback.py +++ b/tests/test_playback.py @@ -1,3 +1,4 @@ +import collections import threading from unittest import mock @@ -64,7 +65,7 @@ def test_connect_events_adds_music_delivery_handler_to_session( playback_provider._seeking_event, playback_provider._push_audio_data_event, playback_provider._buffer_timestamp, - playback_provider._held_buffer, + playback_provider._held_buffers, ) in session_mock.on.call_args_list ) @@ -137,14 +138,14 @@ def test_change_track_clears_state(audio_mock, provider): provider._first_seek = False provider._buffer_timestamp.set(99) provider._end_of_track_event.set() - provider._held_buffer.data = mock.sentinel.gst_buffer + provider._held_buffers = collections.deque([mock.sentinel.gst_buffer]) assert provider.change_track(track) is True assert provider._first_seek assert provider._buffer_timestamp.get() == 0 assert not provider._end_of_track_event.is_set() - assert provider._held_buffer.data is None + assert len(provider._held_buffers) == 0 def test_resume_starts_spotify_playback(session_mock, provider): @@ -225,7 +226,7 @@ def test_music_delivery_rejects_data_when_seeking(session_mock, audio_mock): push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer]) assert seeking_event.is_set() result = playback.music_delivery_callback( @@ -257,7 +258,7 @@ def test_music_delivery_when_seeking_accepts_data_after_empty_delivery( push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer]) assert seeking_event.is_set() result = playback.music_delivery_callback( @@ -287,7 +288,7 @@ def test_music_delivery_rejects_data_depending_on_push_audio_data_event( seeking_event = threading.Event() push_audio_data_event = threading.Event() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer]) assert not push_audio_data_event.is_set() result = playback.music_delivery_callback( @@ -317,7 +318,7 @@ def test_music_delivery_shortcuts_if_no_data_in_frames( push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer]) result = playback.music_delivery_callback( session_mock, @@ -345,7 +346,7 @@ def test_music_delivery_rejects_unknown_audio_formats(session_mock, audio_mock): push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer]) with pytest.raises(AssertionError) as excinfo: playback.music_delivery_callback( @@ -378,7 +379,7 @@ def test_music_delivery_creates_gstreamer_buffer_and_holds_it( push_audio_data_event.set() buffer_timestamp = mock.Mock() buffer_timestamp.get.return_value = mock.sentinel.timestamp - held_buffer = playback.BufferData(None) + held_buffer = collections.deque() result = playback.music_delivery_callback( session_mock, @@ -401,7 +402,7 @@ def test_music_delivery_creates_gstreamer_buffer_and_holds_it( buffer_timestamp.increase.assert_called_once_with(mock.sentinel.duration) audio_mock.emit_data.assert_not_called() assert result == num_frames - assert held_buffer.data == mock.sentinel.gst_buffer + assert list(held_buffer) == [mock.sentinel.gst_buffer] def test_music_delivery_gives_held_buffer_to_audio_and_holds_created( @@ -416,7 +417,7 @@ def test_music_delivery_gives_held_buffer_to_audio_and_holds_created( push_audio_data_event = threading.Event() push_audio_data_event.set() buffer_timestamp = mock.Mock() - held_buffer = playback.BufferData(mock.sentinel.gst_buffer1) + held_buffer = collections.deque([mock.sentinel.gst_buffer1]) result = playback.music_delivery_callback( session_mock, @@ -431,7 +432,7 @@ def test_music_delivery_gives_held_buffer_to_audio_and_holds_created( ) assert result == num_frames audio_mock.emit_data.assert_called_once_with(mock.sentinel.gst_buffer1) - assert held_buffer.data == mock.sentinel.gst_buffer2 + assert list(held_buffer) == [mock.sentinel.gst_buffer2] def test_music_delivery_consumes_zero_frames_if_audio_fails( @@ -449,7 +450,7 @@ def test_music_delivery_consumes_zero_frames_if_audio_fails( push_audio_data_event.set() buffer_timestamp = mock.Mock() buffer_timestamp.get.return_value = mock.sentinel.timestamp - held_buffer = playback.BufferData(mock.sentinel.gst_buffer) + held_buffer = collections.deque([mock.sentinel.gst_buffer1]) result = playback.music_delivery_callback( session_mock, @@ -465,7 +466,7 @@ def test_music_delivery_consumes_zero_frames_if_audio_fails( assert buffer_timestamp.increase.call_count == 0 assert result == 0 - assert held_buffer.data == mock.sentinel.gst_buffer2 # SPONG + assert list(held_buffer) == [mock.sentinel.gst_buffer1] def test_end_of_track_callback(session_mock, audio_mock): From cfd0559cc035a83f9c7c4eed15ebcbac049acbd9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 5 Dec 2020 01:47:00 +0100 Subject: [PATCH 06/11] Move changelog to GitHub Releases --- CHANGELOG.rst | 348 -------------------------------------------------- README.rst | 2 +- 2 files changed, 1 insertion(+), 349 deletions(-) delete mode 100644 CHANGELOG.rst diff --git a/CHANGELOG.rst b/CHANGELOG.rst deleted file mode 100644 index 765de34e..00000000 --- a/CHANGELOG.rst +++ /dev/null @@ -1,348 +0,0 @@ -********* -Changelog -********* - - -v4.0.1 (2020-01-11) -=================== - -- Fix API endpoint for browsing featured playlists. (#252, PR: #253) - - -v4.0.0 (2019-12-22) -=================== - -- Depend on final release of Mopidy 3.0.0. - -- Use the Spotify Web API for personal top lists. (PR: #237) - -- Use the Spotify Web API to support browsing Your Music. (#16, PR: #238) - -- Use the Spotify Web API to support browsing featured playlists. (#240, PR: #247) - -- Use the Spotify web API to fetch playlist images. (#100, PR: #243) - -- Fix fetching of track images when track is relinked. (#171, #244, PR: #245) - - -v4.0.0a2 (2019-12-12) -===================== - -Alpha release. - -- Use the Spotify Web API for playlists. (#122, #133, #140, #156, #182, #213, - #226, PR: #235) - - -v4.0.0a1 (2019-11-18) -===================== - -Alpha release. - -- Require Mopidy >= 3.0.0a5. (PR: #223) - -- Require Python >= 3.7. (PR: #223) - -- Update project setup. (PR: #223) - -- Improved Web API client, including support for retries. (PR: #147) - -- Pause Spotify playback when Mopidy is paused. (#75, #146, PR: #162) - -- Only return search results available in the user's country. (#97, PR: #131) - - -v3.1.0 (2017-06-08) -=================== - -Feature release. - -- Include the artists of each album in the search results. (PR: #118) - -- Respect `spotify/timeout` setting when loading data from Spotify. (#129, PR: - #139) - -- Use OAuth to authenticate Spotify Web API requests, which has required - authentication since 2017-05-29. The new config value `spotify/client_id` and - `spotify/client_secret` must be set. Refer to the README for details. (#130, - #142, PR: #143) - - -v3.0.0 (2016-02-15) -=================== - -Feature release. - -- Require Mopidy 2.0. - -- Minor adjustments to work with GStreamer 1. - - -v2.3.1 (2016-02-14) -=================== - -Bug fix release. - -- Require Mopidy < 2 as Mopidy 2.0 breaks the audio API with the upgrade to - GStreamer 1. - -- Use the new Spotify Web API for search. Searching through libspotify has been - discontinued and is not working anymore. (Fixes: #89) - -- Note that search through the Spotify Web API doesn't return artists or date - for albums. This also means that ``library.get_distinct()`` when given type - ``albumartist`` or ``date`` and a query only returns an empty set. - -- Emit a warning if config value ``spotify/search_album_count``, - ``spotify/search_artist_count``, or ``spotify/search_track_count`` is greater - than 50, and use 50 instead. 50 is the maximum value that the Spotify Web API - allows. The maximum in the config schema is not changed to not break existing - configs. - - -v2.3.0 (2016-02-06) -=================== - -Feature release. - -- Ignore all audio data deliveries from libspotify when when a seek is in - progress. This ensures that we don't deliver audio data from before the seek - with timestamps from after the seek. - -- Ignore duplicate end of track callbacks. - -- Don't increase the audio buffer timestamp if the buffer is rejected by - Mopidy. This caused audio buffers delivered after one or more rejected audio - buffers to have too high timestamps. - -- When changing tracks, block until Mopidy completes the appsrc URI change. - Not blocking here might break gapless playback. - -- Lookup of a playlist you're not subscribed to will now properly load all of - the playlist's tracks. (Fixes: #81, PR: #82) - -- Workaround teardown race outputing lots of short stack traces on Mopidy - shutdown. (See #73 for details) - - -v2.2.0 (2015-11-15) -=================== - -Feature release. - -- As Spotify now exposes your "Starred" playlist as a regular playlist, we no - longer get your "Starred" playlist through the dedicated "Starred" API. This - avoids your "Starred" playlist being returned twice. Lookup of - ``spotify:user::starred`` URIs works as before. (Fixes: #71) - -- Interpret album year "0" as unknown. (Fixes: #72) - - -v2.1.0 (2015-09-22) -=================== - -Feature release. - -- Require Requests >= 2.0. - -- Support using a proxy when connecting to Spotify. This was forgotten in the - 2.0 reimplementation. (Fixes: #68) - -- Support using a proxy when accessing Spotify's web API to get cover and - artist imagery. - - -v2.0.1 (2015-08-23) -=================== - -Bug fix release. - -- Filter out ``None`` from ``library.get_distinct()`` return values. (Fixes: - #63) - - -v2.0.0 (2015-08-11) -=================== - -Rewrite using pyspotify 2. Should have feature parity with Mopidy-Spotify 1. - -**Config** - -- Add ``spotify/volume_normalization`` config. (Fixes: #13) - -- Add ``spotify/allow_network`` config which can be used to force - Mopidy-Spotify to stay offline. This is mostly useful for testing during - development. - -- Add ``spotify/allow_playlists`` config which can be used to disable all - access to playlists on the Spotify account. Useful where Mopidy is shared by - multiple users. (Fixes: #25) - -- Make maximum number of returned results configurable through - ``spotify/search_album_count``, ``spotify/search_artist_count``, and - ``spotify/search_track_count``. - -- Add ``spotify/private_session`` config. - -- Change ``spotify/toplist_countries`` default value to blank, which is now - interpreted as all supported countries instead of no countries. - -- Removed ``spotify/cache_dir`` and ``spotify/settings_dir`` config values. We - now use a "spotify" directory in the ``core/cache_dir`` and - ``core/data_dir`` directories defined in Mopidy's configuration. - -- Add ``spotify/allow_cache`` config value to make it possible to disable - caching. - -**Browse** - -- Add browsing of top albums and top artists, in additon to top tracks. - -- Add browsing by current user's country, in addition to personal, global and - per-country browsing. - -- Add browsing of artists, which includes the artist's top tracks and albums. - -- Update list of countries Spotify is available in and provides toplists for. - -**Lookup** - -- Adding an artist by URI will now first find all albums by the artist and - then all tracks in the albums. This way, the returned tracks are grouped by - album and they are sorted by track number. (Fixes: #7) - -- When adding an artist by URI, all albums that are marked as "compilations" - or where the album artist is "Various Artists" are now ignored. (Fixes: #5) - -**Library** - -- The library provider method ``get_distinct()`` is now supported. When called - without a query, the tracks in the user's playlists is used as the data - source. When called with a query, a Spotify search is used as the data - source. This addition makes the library view in some notable MPD clients, - like ncmpcpp, become quite fast and usable with Spotify. (Fixes: #50) - -**Playback** - -- If another Spotify client starts playback with the same account, we get a - "play token lost" event. Previously, Mopidy-Spotify would unconditionally - pause Mopidy playback if this happened. Now, we only pause playback if we're - currently playing music from Spotify. (Fixes: #1) - - -v1.4.0 (2015-05-19) -=================== - -- Update to not use deprecated Mopidy audio APIs. - -- Use strings and not ints for the model's date field. This is required for - compatibility with the model validation added in Mopidy 1.1. (Fixes: #52) - -- Fix error causing the image of every 50th URI in a ``library.get_images()`` - call to not be looked up and returned. - -- Fix handling of empty search queries. This was still using the removed - ``playlists.playlists`` to fetch all your tracks. - -- Update the ``SpotifyTrack`` proxy model to work with Mopidy 1.1 model - changes. - -- Updated to work with the renaming of ``mopidy.utils`` to ``mopidy.internal`` - in Mopidy 1.1. - - -v1.3.0 (2015-03-25) -=================== - -- Require Mopidy >= 1.0. - -- Update to work with new playback API in Mopidy 1.0. - -- Update to work with new playlists API in Mopidy 1.0. - -- Update to work with new search API in Mopidy 1.0. - -- Add ``library.get_images()`` support for cover art. - - -v1.2.0 (2014-07-21) -=================== - -- Add support for browsing playlists and albums. Needed to allow music - discovery extensions expose these in a clean way. - -- Fix loss of audio when resuming from paused, when caused by another Spotify - client starting playback. (Fixes: #2, PR: #19) - - -v1.1.3 (2014-02-18) -=================== - -- Switch to new backend API locations, required by the upcoming Mopidy 0.19 - release. - - -v1.1.2 (2014-02-18) -=================== - -- Wait for track to be loaded before playing it. This fixes playback of tracks - looked up directly by URI, and not through a playlist or search. (Fixes: - mopidy/mopidy#675) - - -v1.1.1 (2014-02-16) -=================== - -- Change requirement on pyspotify from ``>= 1.9, < 2`` to ``>= 1.9, < 1.999``, - so that it is parsed correctly and pyspotify 1.x is installed instead of 2.x. - - -v1.1.0 (2014-01-20) -=================== - -- Require Mopidy >= 0.18. - -- Change ``library.lookup()`` to return tracks even if they are unplayable. - There's no harm in letting them be added to the tracklist, as Mopidy will - simply skip to the next track when failing to play the track. (Fixes: - mopidy/mopidy#606) - -- Added basic library browsing support that exposes user, global and country - toplists. - - -v1.0.3 (2013-12-15) -=================== - -- Change search field ``track`` to ``track_name`` for compatibility with - Mopidy 0.17. (Fixes: mopidy/mopidy#610) - - -v1.0.2 (2013-11-19) -=================== - -- Add ``spotify/settings_dir`` config value so that libspotify settings can be - stored to another location than the libspotify cache. This also allows - ``spotify/cache_dir`` to be unset, since settings are now using it's own - config value. - -- Make the ``spotify/cache_dir`` config value optional, so that it can be set - to an empty string to disable caching. - - -v1.0.1 (2013-10-28) -=================== - -- Support searches from Mopidy that are using the ``albumartist`` field type, - added in Mopidy 0.16. - -- Ignore the ``track_no`` field in search queries, added in Mopidy 0.16. - -- Abort Spotify searches immediately if the search query is empty instead of - waiting for the 10s timeout before returning an empty search result. - - -v1.0.0 (2013-10-08) -=================== - -- Moved extension out of the main Mopidy project. diff --git a/README.rst b/README.rst index 865a20e5..2dfa1a6f 100644 --- a/README.rst +++ b/README.rst @@ -169,7 +169,7 @@ Project resources - `Source code `_ - `Issue tracker `_ -- `Changelog `_ +- `Changelog `_ Credits From 8edfb1adaf3a127d26992ca713c38aee1ec83160 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 5 Dec 2020 01:47:17 +0100 Subject: [PATCH 07/11] Use GitHub Actions to release to PyPI --- .github/workflows/release.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..f9ae1405 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,23 @@ +name: Release + +on: + release: + types: [published] + +jobs: + release: + runs-on: ubuntu-20.04 + + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + with: + python-version: '3.9' + - name: "Install dependencies" + run: python3 -m pip install wheel + - name: "Build package" + run: python3 setup.py sdist bdist_wheel + - uses: pypa/gh-action-pypi-publish@v1.4.1 + with: + user: __token__ + password: ${{ secrets.PYPI_TOKEN }} From 3b2306081b89acc486b977d20b427068aa72eef3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 5 Dec 2020 01:52:41 +0100 Subject: [PATCH 08/11] Support Python 3.9 --- .circleci/config.yml | 10 ++++++++-- setup.cfg | 1 + tox.ini | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bfd4689a..d2258839 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,6 +7,7 @@ workflows: version: 2 test: jobs: + - py39 - py38 - py37 - black @@ -14,9 +15,9 @@ workflows: - flake8 jobs: - py38: &test-template + py39: &test-template docker: - - image: mopidy/ci-python:3.8 + - image: mopidy/ci-python:3.9 steps: - checkout - restore_cache: @@ -39,6 +40,11 @@ jobs: - store_test_results: path: test-results + py38: + <<: *test-template + docker: + - image: mopidy/ci-python:3.8 + py37: <<: *test-template docker: diff --git a/setup.cfg b/setup.cfg index b032e3db..f179cce8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,6 +16,7 @@ classifiers = Programming Language :: Python :: 3 Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 + Programming Language :: Python :: 3.9 Topic :: Multimedia :: Sound/Audio :: Players diff --git a/tox.ini b/tox.ini index c99a3999..b11b5ee9 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py37, py38, black, check-manifest, flake8 +envlist = py37, py38, py39, black, check-manifest, flake8 [testenv] sitepackages = true From 915cf34e67d6eb9b9a1f1be041c0cda173bd274f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 5 Dec 2020 02:00:20 +0100 Subject: [PATCH 09/11] Release v4.1.0 --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f179cce8..ebf029d5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = Mopidy-Spotify -version = 4.0.1 +version = 4.1.0 url = https://github.com/mopidy/mopidy-spotify author = Stein Magnus Jodal author_email = stein.magnus@jodal.no From a4aa63adb291d0c3a6f5af53fe466564b2e4c878 Mon Sep 17 00:00:00 2001 From: Nick Steel Date: Fri, 4 Dec 2020 12:30:23 +0000 Subject: [PATCH 10/11] Ignore parts of a Web API response with uri value None. We can't do anything sensible without a URI so ignore anything that doesn't have one. This occurs in the album and artist fields within local Spotify tracks (from a playlist). --- mopidy_spotify/translator.py | 2 +- tests/test_translator.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/mopidy_spotify/translator.py b/mopidy_spotify/translator.py index 4ba2fcfc..956394fa 100644 --- a/mopidy_spotify/translator.py +++ b/mopidy_spotify/translator.py @@ -172,7 +172,7 @@ def valid_web_data(data, object_type): return ( isinstance(data, dict) and data.get("type") == object_type - and "uri" in data + and data.get("uri") is not None ) diff --git a/tests/test_translator.py b/tests/test_translator.py index 2cd62561..e12ae9e0 100644 --- a/tests/test_translator.py +++ b/tests/test_translator.py @@ -300,6 +300,10 @@ def test_returns_false_if_missing_uri(self, web_track_mock): del web_track_mock["uri"] assert translator.valid_web_data(web_track_mock, "track") is False + def test_return_false_if_uri_none(self, web_track_mock): + web_track_mock["uri"] = None + assert translator.valid_web_data(web_track_mock, "track") is False + def test_returns_success(self, web_track_mock): assert translator.valid_web_data(web_track_mock, "track") is True @@ -716,6 +720,22 @@ def test_successful_translation(self, web_album_mock): assert album.name == "DEF 456" assert list(album.artists) == artists + def test_ignores_invalid_artists(self, web_album_mock, web_artist_mock): + invalid_artist1 = {"name": "FOO", "uri": None, "type": "artist"} + invalid_artist2 = {"name": "BAR", "type": "football"} + web_album_mock["artists"] = [ + invalid_artist1, + web_artist_mock, + invalid_artist2, + ] + album = translator.web_to_album(web_album_mock) + + artists = [models.Artist(uri="spotify:artist:abba", name="ABBA")] + + assert album.uri == "spotify:album:def" + assert album.name == "DEF 456" + assert list(album.artists) == artists + class TestWebToTrack: def test_calls_web_to_track_ref(self, web_track_mock): @@ -773,3 +793,11 @@ def test_ignores_missing_album(self, web_track_mock): assert track.name == "ABC 123" assert track.length == 174300 assert track.album is None + + def test_ignores_invalid_album(self, web_track_mock): + web_track_mock["album"]["uri"] = None + + track = translator.web_to_track(web_track_mock) + + assert track.name == "ABC 123" + assert track.album is None From ea782020993fb0a33e6628bf7e12d9e45069e1b4 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 6 Dec 2020 12:03:30 +0100 Subject: [PATCH 11/11] Use 'build' to build independently of setuptools --- .github/workflows/release.yml | 4 ++-- setup.cfg | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f9ae1405..21a6ea1e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,9 +14,9 @@ jobs: with: python-version: '3.9' - name: "Install dependencies" - run: python3 -m pip install wheel + run: python3 -m pip install build - name: "Build package" - run: python3 setup.py sdist bdist_wheel + run: python3 -m build - uses: pypa/gh-action-pypi-publish@v1.4.1 with: user: __token__ diff --git a/setup.cfg b/setup.cfg index ebf029d5..bf23e39f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -42,8 +42,8 @@ lint = flake8-import-order isort[pyproject] release = + build twine - wheel test = pytest pytest-cov