From 2234a04fc7e1507307f3f70dc825188cca289375 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Mar 2015 21:27:08 +0200 Subject: [PATCH 1/7] audio: Make outputs helper only handle tee-ing. The queue which is needed for gapless has been moved up to a audio-sink bin which also wraps the outputs. --- mopidy/audio/actor.py | 46 +++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index b4c78ecb..3d8c8ef8 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -152,26 +152,12 @@ class _Appsrc(object): # TODO: expose this as a property on audio when #790 gets further along. class _Outputs(gst.Bin): def __init__(self): - gst.Bin.__init__(self) + gst.Bin.__init__(self, 'outputs') self._tee = gst.element_factory_make('tee') self.add(self._tee) - # Queue element to buy us time between the about to finish event and - # the actual switch, i.e. about to switch can block for longer thanks - # to this queue. - # TODO: make the min-max values a setting? - # TODO: this does not belong in this class. - queue = gst.element_factory_make('queue') - queue.set_property('max-size-buffers', 0) - queue.set_property('max-size-bytes', 0) - queue.set_property('max-size-time', 5 * gst.SECOND) - queue.set_property('min-threshold-time', 3 * gst.SECOND) - self.add(queue) - - queue.link(self._tee) - - ghost_pad = gst.GhostPad('sink', queue.get_pad('sink')) + ghost_pad = gst.GhostPad('sink', self._tee.get_pad('sink')) self.add_pad(ghost_pad) # Add an always connected fakesink which respects the clock so the tee @@ -451,8 +437,9 @@ class Audio(pykka.ThreadingActor): try: self._setup_preferences() self._setup_playbin() - self._setup_output() + self._setup_outputs() self._setup_mixer() + self._setup_audio_sink() except gobject.GError as ex: logger.exception(ex) process.exit_process() @@ -492,7 +479,7 @@ class Audio(pykka.ThreadingActor): self._signals.disconnect(self._playbin, 'source-setup') self._playbin.set_state(gst.STATE_NULL) - def _setup_output(self): + def _setup_outputs(self): # We don't want to use outputs for regular testing, so just install # an unsynced fakesink when someone asks for a 'testoutput'. if self._config['audio']['output'] == 'testoutput': @@ -505,12 +492,33 @@ class Audio(pykka.ThreadingActor): process.exit_process() # TODO: move this up the chain self._handler.setup_event_handling(self._outputs.get_pad('sink')) - self._playbin.set_property('audio-sink', self._outputs) def _setup_mixer(self): if self.mixer: self.mixer.setup(self._playbin, self.actor_ref.proxy().mixer) + def _setup_audio_sink(self): + audio_sink = gst.Bin('audio-sink') + + # Queue element to buy us time between the about to finish event and + # the actual switch, i.e. about to switch can block for longer thanks + # to this queue. + # TODO: make the min-max values a setting? + queue = gst.element_factory_make('queue') + queue.set_property('max-size-buffers', 0) + queue.set_property('max-size-bytes', 0) + queue.set_property('max-size-time', 5 * gst.SECOND) + queue.set_property('min-threshold-time', 3 * gst.SECOND) + + audio_sink.add(queue) + audio_sink.add(self._outputs) + queue.link(self._outputs) + + ghost_pad = gst.GhostPad('sink', queue.get_pad('sink')) + audio_sink.add_pad(ghost_pad) + + self._playbin.set_property('audio-sink', audio_sink) + def _teardown_mixer(self): if self.mixer: self.mixer.teardown() From 8236417e9d2c4d9c400b60dcbc4714de52b2bcbe Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 2 Apr 2015 23:26:05 +0200 Subject: [PATCH 2/7] audio: Move software volume into audiosink. This turns off playbin controlled volume, which implies that pulsesink volume can no longer be controlled by Mopidy. This is likely something we have to break, or at least rethink for multiple output support any way. With this change we now have software volume after our large queue, which means volume changes should happen much faster. --- mopidy/audio/actor.py | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 3d8c8ef8..66718368 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -36,23 +36,6 @@ _GST_STATE_MAPPING = { MB = 1 << 20 -# GST_PLAY_FLAG_VIDEO (1<<0) -# GST_PLAY_FLAG_AUDIO (1<<1) -# GST_PLAY_FLAG_TEXT (1<<2) -# GST_PLAY_FLAG_VIS (1<<3) -# GST_PLAY_FLAG_SOFT_VOLUME (1<<4) -# GST_PLAY_FLAG_NATIVE_AUDIO (1<<5) -# GST_PLAY_FLAG_NATIVE_VIDEO (1<<6) -# GST_PLAY_FLAG_DOWNLOAD (1<<7) -# GST_PLAY_FLAG_BUFFERING (1<<8) -# GST_PLAY_FLAG_DEINTERLACE (1<<9) -# GST_PLAY_FLAG_SOFT_COLORBALANCE (1<<10) - -# Default flags to use for playbin: AUDIO, SOFT_VOLUME -# TODO: consider removing soft volume when we do multi outputs and handling it -# ourselves. -PLAYBIN_FLAGS = (1 << 1) | (1 << 4) - class _Signals(object): """Helper for tracking gobject signal registrations""" @@ -438,7 +421,6 @@ class Audio(pykka.ThreadingActor): self._setup_preferences() self._setup_playbin() self._setup_outputs() - self._setup_mixer() self._setup_audio_sink() except gobject.GError as ex: logger.exception(ex) @@ -459,7 +441,7 @@ class Audio(pykka.ThreadingActor): def _setup_playbin(self): playbin = gst.element_factory_make('playbin2') - playbin.set_property('flags', PLAYBIN_FLAGS) + playbin.set_property('flags', 2) # GST_PLAY_FLAG_AUDIO # TODO: turn into config values... playbin.set_property('buffer-size', 2 * 1024 * 1024) @@ -493,10 +475,6 @@ class Audio(pykka.ThreadingActor): self._handler.setup_event_handling(self._outputs.get_pad('sink')) - def _setup_mixer(self): - if self.mixer: - self.mixer.setup(self._playbin, self.actor_ref.proxy().mixer) - def _setup_audio_sink(self): audio_sink = gst.Bin('audio-sink') @@ -512,7 +490,15 @@ class Audio(pykka.ThreadingActor): audio_sink.add(queue) audio_sink.add(self._outputs) - queue.link(self._outputs) + + if self.mixer: + volume = gst.element_factory_make('volume') + audio_sink.add(volume) + queue.link(volume) + volume.link(self._outputs) + self.mixer.setup(volume, self.actor_ref.proxy().mixer) + else: + queue.link(self._outputs) ghost_pad = gst.GhostPad('sink', queue.get_pad('sink')) audio_sink.add_pad(ghost_pad) From e76c3c90120df63a3cb06dd42a222f44f2e23e78 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 2 Apr 2015 23:35:15 +0200 Subject: [PATCH 3/7] audio: Remove notify::mute/volume from software mixer These will never be triggered externally when using plain software volume. --- mopidy/audio/actor.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 66718368..77b64d58 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -183,10 +183,6 @@ class SoftwareMixer(object): def setup(self, element, mixer_ref): self._element = element - - self._signals.connect(element, 'notify::volume', self._volume_changed) - self._signals.connect(element, 'notify::mute', self._mute_changed) - self._mixer.setup(mixer_ref) def teardown(self): @@ -198,24 +194,16 @@ class SoftwareMixer(object): def set_volume(self, volume): self._element.set_property('volume', volume / 100.0) + self._mixer.trigger_volume_changed(volume) def get_mute(self): return self._element.get_property('mute') def set_mute(self, mute): - return self._element.set_property('mute', bool(mute)) - - def _volume_changed(self, element, property_): - old_volume, self._last_volume = self._last_volume, self.get_volume() - if old_volume != self._last_volume: - gst_logger.debug('Notify volume: %s', self._last_volume / 100.0) - self._mixer.trigger_volume_changed(self._last_volume) - - def _mute_changed(self, element, property_): - old_mute, self._last_mute = self._last_mute, self.get_mute() - if old_mute != self._last_mute: - gst_logger.debug('Notify mute: %s', self._last_mute) - self._mixer.trigger_mute_changed(self._last_mute) + result = self._element.set_property('mute', bool(mute)) + if result: + self._mixer.trigger_mute_changed(bool(mute)) + return result class _Handler(object): From 9f90b37aa53748302a3fc9fb3f39b8c9f3438e0d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 2 Apr 2015 23:40:58 +0200 Subject: [PATCH 4/7] audio: Limit post tee queue size Not sure how small we can safely make this, but basically with the volume element in front of the tee we "need" this as small as possible so the volume changes fell snappy. Alternative would be one volume element per tee branch. --- mopidy/audio/actor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 77b64d58..b4e5ebbc 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -164,7 +164,9 @@ class _Outputs(gst.Bin): def _add(self, element): # All tee branches need a queue in front of them. + # But keep the queue short so the volume change isn't to slow: queue = gst.element_factory_make('queue') + queue.set_property('max-size-buffers', 5) self.add(element) self.add(queue) queue.link(element) From db48845e91212d1a0f5577b06f0fdf7fbd390023 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 2 Apr 2015 23:48:27 +0200 Subject: [PATCH 5/7] audio: Adjust queue sizes. These are mostly just gut feeling guesses. We should really start exposing at least a few of these as settings soon. --- mopidy/audio/actor.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index b4e5ebbc..35bd215f 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -34,8 +34,6 @@ _GST_STATE_MAPPING = { gst.STATE_PAUSED: PlaybackState.PAUSED, gst.STATE_NULL: PlaybackState.STOPPED} -MB = 1 << 20 - class _Signals(object): """Helper for tracking gobject signal registrations""" @@ -97,7 +95,7 @@ class _Appsrc(object): source.set_property('caps', self._caps) source.set_property('format', b'time') source.set_property('stream-type', b'seekable') - source.set_property('max-bytes', 1 * MB) + source.set_property('max-bytes', 1 << 20) # 1MB source.set_property('min-percent', 50) if self._need_data_callback: @@ -434,8 +432,8 @@ class Audio(pykka.ThreadingActor): playbin.set_property('flags', 2) # GST_PLAY_FLAG_AUDIO # TODO: turn into config values... - playbin.set_property('buffer-size', 2 * 1024 * 1024) - playbin.set_property('buffer-duration', 2 * gst.SECOND) + playbin.set_property('buffer-size', 5 << 20) # 5MB + playbin.set_property('buffer-duration', 5 * gst.SECOND) self._signals.connect(playbin, 'source-setup', self._on_source_setup) self._signals.connect(playbin, 'about-to-finish', @@ -475,8 +473,8 @@ class Audio(pykka.ThreadingActor): queue = gst.element_factory_make('queue') queue.set_property('max-size-buffers', 0) queue.set_property('max-size-bytes', 0) - queue.set_property('max-size-time', 5 * gst.SECOND) - queue.set_property('min-threshold-time', 3 * gst.SECOND) + queue.set_property('max-size-time', 3 * gst.SECOND) + queue.set_property('min-threshold-time', 1 * gst.SECOND) audio_sink.add(queue) audio_sink.add(self._outputs) From bee0a4c4d5fdc0843eeb8a71ac72688af196ea32 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 2 Apr 2015 23:59:46 +0200 Subject: [PATCH 6/7] docs: Add audio volume changes to changelog --- docs/changelog.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b976c169..01a5dc6d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,19 @@ Changelog This changelog is used to track all major changes to Mopidy. +v1.0.1 (unreleased) +=================== + +Audio +----- + +- Software volume control has been reworked to greatly reduce the delay between + changing the volume and the change taking effect. (Fixes: :issue:`1097`) + +- Software volume is no longer tied to the PulseAudio application volume when + using ``pulsesink``. This behavior was confusing for many users and doesn't + work well with the plans for multiple outputs. + v1.0.0 (2015-03-25) =================== From 5d94a265cd240c1f0eab43286e1dae7848442c48 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 5 Apr 2015 02:13:30 +0200 Subject: [PATCH 7/7] docs: Tweak changelog --- docs/changelog.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 01a5dc6d..ce7be87b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,18 +5,18 @@ Changelog This changelog is used to track all major changes to Mopidy. -v1.0.1 (unreleased) +v1.0.1 (UNRELEASED) =================== -Audio ------ +- Audio: Software volume control has been reworked to greatly reduce the delay + between changing the volume and the change taking effect. (Fixes: + :issue:`1097`) -- Software volume control has been reworked to greatly reduce the delay between - changing the volume and the change taking effect. (Fixes: :issue:`1097`) +- Audio: As a side effect of the previous bug fix, software volume is no longer + tied to the PulseAudio application volume when using ``pulsesink``. This + behavior was confusing for many users and doesn't work well with the plans + for multiple outputs. -- Software volume is no longer tied to the PulseAudio application volume when - using ``pulsesink``. This behavior was confusing for many users and doesn't - work well with the plans for multiple outputs. v1.0.0 (2015-03-25) ===================