From 4d287697e42a53575fc6be34af9211851f728d37 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sat, 29 Mar 2014 15:10:56 +0100 Subject: [PATCH 1/4] Pause gstreamer when buffering When the `playbin2` pipeline is in `PLAYING` state and its buffer is empty the pipeline will consume arriving data immediately. If the source is an HTTP stream this leads to choppy playback. To fix this we pause playing when the buffer is nearly empty and wait for it to fill up until we resume playing. This approach is recommended in the gstreamer manual [1]. We might want to extract the hard-coded 10% mark and make it configurable. [1]: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/manual/html/chapter-buffering.html --- mopidy/audio/actor.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index ca023125..93b2769e 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -278,6 +278,10 @@ class Audio(pykka.ThreadingActor): self._on_playbin_state_changed(old_state, new_state, pending_state) elif message.type == gst.MESSAGE_BUFFERING: percent = message.parse_buffering() + if percent < 10: + self._playbin.set_state(gst.STATE_PAUSED) + if percent == 100: + self._playbin.set_state(gst.STATE_PLAYING) logger.debug('Buffer %d%% full', percent) elif message.type == gst.MESSAGE_EOS: self._on_end_of_stream() From d42ebd1feceb3abcaa6b594e1923e9793da3994a Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sat, 29 Mar 2014 15:22:34 +0100 Subject: [PATCH 2/4] Set buffer properties of gstreamer pipeline Manually set the `playbin2` buffer properties to their default values. The default values are not documented but hard-coded in gstreamer [1]. This is a starting point for making these properties configurable. For example we might want higher values on system with unsteady network connection. [1]: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/playback/gsturidecodebin.c?h=0.10#n1714 --- mopidy/audio/actor.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 93b2769e..8dfe0fa8 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -100,6 +100,9 @@ class Audio(pykka.ThreadingActor): playbin = gst.element_factory_make('playbin2') playbin.set_property('flags', PLAYBIN_FLAGS) + playbin.set_property('buffer-size', 2*1024*1024) + playbin.set_property('buffer-duration', 2*gst.SECOND) + self._connect(playbin, 'about-to-finish', self._on_about_to_finish) self._connect(playbin, 'notify::source', self._on_new_source) From 1e16817e390f2156964f0bb20393ab62a3a804dc Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 20:30:41 +0200 Subject: [PATCH 3/4] =?UTF-8?q?Don=E2=80=99t=20start=20playing=20automatic?= =?UTF-8?q?ally=20when=20buffer=20is=20full?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when `Audio.pause_playback()` was called while buffering, the audio actor would switch back to the playing state when the buffer was full. --- mopidy/audio/actor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 8dfe0fa8..41bcec94 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -49,6 +49,7 @@ class Audio(pykka.ThreadingActor): #: The GStreamer state mapped to :class:`mopidy.audio.PlaybackState` state = PlaybackState.STOPPED + _target_state = gst.STATE_NULL def __init__(self, config): super(Audio, self).__init__() @@ -283,7 +284,7 @@ class Audio(pykka.ThreadingActor): percent = message.parse_buffering() if percent < 10: self._playbin.set_state(gst.STATE_PAUSED) - if percent == 100: + if percent == 100 and self._target_state == gst.STATE_PLAYING: self._playbin.set_state(gst.STATE_PLAYING) logger.debug('Buffer %d%% full', percent) elif message.type == gst.MESSAGE_EOS: @@ -477,6 +478,7 @@ class Audio(pykka.ThreadingActor): :type state: :class:`gst.State` :rtype: :class:`True` if successfull, else :class:`False` """ + self._target_state = state result = self._playbin.set_state(state) if result == gst.STATE_CHANGE_FAILURE: logger.warning( From 2b7ff237b843e9dd6a18e79bc202c10ec6fe99b1 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 3 Apr 2014 21:43:56 +0200 Subject: [PATCH 4/4] Add tests for Audio buffering --- tests/audio/test_actor.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 8263d769..69d855e7 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +from mock import Mock import unittest import pygst @@ -158,3 +159,35 @@ class AudioStateTest(unittest.TestCase): # gst.STATE_READY, gst.STATE_NULL, gst.STATE_VOID_PENDING) self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) + + +class AudioBufferingTest(unittest.TestCase): + def setUp(self): + self.audio = audio.Audio(config=None) + self.audio._playbin = Mock(spec=['set_state']) + + self.buffer_full_message = Mock() + self.buffer_full_message.type = gst.MESSAGE_BUFFERING + self.buffer_full_message.parse_buffering = Mock(return_value=100) + + self.buffer_empty_message = Mock() + self.buffer_empty_message.type = gst.MESSAGE_BUFFERING + self.buffer_empty_message.parse_buffering = Mock(return_value=0) + + def test_pause_when_buffer_empty(self): + playbin = self.audio._playbin + self.audio.start_playback() + playbin.set_state.assert_called_with(gst.STATE_PLAYING) + playbin.set_state.reset_mock() + + self.audio._on_message(None, self.buffer_empty_message) + playbin.set_state.assert_called_with(gst.STATE_PAUSED) + + def test_stay_paused_when_buffering_finished(self): + playbin = self.audio._playbin + self.audio.pause_playback() + playbin.set_state.assert_called_with(gst.STATE_PAUSED) + playbin.set_state.reset_mock() + + self.audio._on_message(None, self.buffer_full_message) + self.assertEqual(playbin.set_state.call_count, 0)