Needs a commit message

Author: kingosticks

mopidy_spotify/playback.py

@@ -1,4 +1,5 @@
 import functools
+import collections
 import logging
 import threading
 
@@ -33,7 +34,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()
+        self._held_buffers = collections.deque()
 
     def _connect_events(self):
         if not self._events_connected:
@@ -45,7 +46,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 +81,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 +169,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.
@@ -199,21 +200,25 @@ def music_delivery_callback(
     )
 
     # 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()
+    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:
+        # Held buffer consumed, holding the next buffer. This is to drop the last 
+        # buffer we receieve for the track which does not contain audio data. 
+        held_buffers.append(buffer_)
         buffer_timestamp.increase(duration)
         return num_frames
     else:
+        # Pass back-pressure on to libspotify, new buffer will be redelivered
         return 0
 
 
@@ -254,10 +259,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

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):