From fe9ad74e1b12825d999bd2631b3230bbdeb060b5 Mon Sep 17 00:00:00 2001 From: Johannes Knutsen Date: Sat, 14 Aug 2010 15:44:05 +0200 Subject: [PATCH 1/8] fixed test_seek and test_seekid --- tests/frontends/mpd/playback_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/frontends/mpd/playback_test.py b/tests/frontends/mpd/playback_test.py index 42525d90..60e96d88 100644 --- a/tests/frontends/mpd/playback_test.py +++ b/tests/frontends/mpd/playback_test.py @@ -271,17 +271,17 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) def test_seek(self): - self.b.current_playlist.load([Track()]) + self.b.current_playlist.load([Track(length=40000)]) self.h.handle_request(u'seek "0"') result = self.h.handle_request(u'seek "0" "30"') self.assert_(u'OK' in result) - self.assert_(self.b.playback.time_position > 30000) + self.assert_(self.b.playback.time_position >= 30000) def test_seekid(self): - self.b.current_playlist.load([Track()]) - result = self.h.handle_request(u'seekid "0" "30"') + self.b.current_playlist.load([Track(length=40000)]) + result = self.h.handle_request(u'seekid "1" "30"') self.assert_(u'OK' in result) - self.assert_(self.b.playback.time_position > 30000) + self.assert_(self.b.playback.time_position >= 30000) def test_stop(self): From 8599bcd491d66644c6f2be67d31097c52d3ed5de Mon Sep 17 00:00:00 2001 From: Johannes Knutsen Date: Sat, 14 Aug 2010 15:50:15 +0200 Subject: [PATCH 2/8] test seekid updates cpid --- tests/frontends/mpd/playback_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/frontends/mpd/playback_test.py b/tests/frontends/mpd/playback_test.py index 60e96d88..d347308c 100644 --- a/tests/frontends/mpd/playback_test.py +++ b/tests/frontends/mpd/playback_test.py @@ -283,6 +283,13 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) self.assert_(self.b.playback.time_position >= 30000) + def test_seekid_with_cpid(self): + seek_track = Track(uri='2', length=40000) + self.b.current_playlist.load( + [Track(length=40000), seek_track]) + result = self.h.handle_request(u'seekid "2" "30"') + self.assertEqual(self.b.playback.current_cpid, 2) + self.assertEqual(self.b.playback.current_track, seek_track) def test_stop(self): result = self.h.handle_request(u'stop') From 00f59e590b58339770276ba7c149ce74f42f5f93 Mon Sep 17 00:00:00 2001 From: Johannes Knutsen Date: Sat, 14 Aug 2010 16:00:06 +0200 Subject: [PATCH 3/8] test seek with songpos --- tests/frontends/mpd/playback_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/frontends/mpd/playback_test.py b/tests/frontends/mpd/playback_test.py index d347308c..a1331bb3 100644 --- a/tests/frontends/mpd/playback_test.py +++ b/tests/frontends/mpd/playback_test.py @@ -277,6 +277,13 @@ class PlaybackControlHandlerTest(unittest.TestCase): self.assert_(u'OK' in result) self.assert_(self.b.playback.time_position >= 30000) + def test_seek_with_songpos(self): + seek_track = Track(uri='2', length=40000) + self.b.current_playlist.load( + [Track(uri='1', length=40000), seek_track]) + result = self.h.handle_request(u'seek "1" "30"') + self.assertEqual(self.b.playback.current_track, seek_track) + def test_seekid(self): self.b.current_playlist.load([Track(length=40000)]) result = self.h.handle_request(u'seekid "1" "30"') From 473c31fec8fa55004dca449e5854c810ac68a554 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 16:42:46 +0200 Subject: [PATCH 4/8] Revert "Fix set_state method in GStreamerOutput" This reverts commit dc27434df9e66e530247fd47270729618be65a8d. --- mopidy/outputs/gstreamer.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/outputs/gstreamer.py b/mopidy/outputs/gstreamer.py index 5e64378f..a28329f7 100644 --- a/mopidy/outputs/gstreamer.py +++ b/mopidy/outputs/gstreamer.py @@ -159,10 +159,10 @@ class GStreamerProcess(BaseProcess): :type state_name: string :rtype: :class:`True` or :class:`False` """ - state = getattr(gst, 'STATE_' + state_name) - self.gst_pipeline.set_state(state) - new_state = self.gst_pipeline.get_state()[1] - if new_state == state: + # XXX Setting state to PLAYING often returns False even if it works + result = self.gst_pipeline.set_state( + getattr(gst, 'STATE_' + state_name)) + if result == gst.STATE_CHANGE_SUCCESS: logger.debug('Setting GStreamer state to %s: OK', state_name) return True else: From 97ff6bf04293e9d6d9e6ed6247287e05da1ef0b0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 17:24:06 +0200 Subject: [PATCH 5/8] Check if failure instead of chacking for success when setting GStreamer state --- mopidy/outputs/gstreamer.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mopidy/outputs/gstreamer.py b/mopidy/outputs/gstreamer.py index a28329f7..44e12afb 100644 --- a/mopidy/outputs/gstreamer.py +++ b/mopidy/outputs/gstreamer.py @@ -159,15 +159,14 @@ class GStreamerProcess(BaseProcess): :type state_name: string :rtype: :class:`True` or :class:`False` """ - # XXX Setting state to PLAYING often returns False even if it works result = self.gst_pipeline.set_state( getattr(gst, 'STATE_' + state_name)) - if result == gst.STATE_CHANGE_SUCCESS: - logger.debug('Setting GStreamer state to %s: OK', state_name) - return True - else: + if result == gst.STATE_CHANGE_FAILURE: logger.warning('Setting GStreamer state to %s: failed', state_name) return False + else: + logger.debug('Setting GStreamer state to %s: OK', state_name) + return True def get_volume(self): """Get volume in range [0..100]""" From 4205a3d49bb70c715a6c852a41b4328c6fc2de86 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 17:49:17 +0200 Subject: [PATCH 6/8] Remove recalibration at volume=0 with NadMixer. It's just way to frustrating. --- mopidy/mixers/nad.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/mopidy/mixers/nad.py b/mopidy/mixers/nad.py index 56958005..d78863aa 100644 --- a/mopidy/mixers/nad.py +++ b/mopidy/mixers/nad.py @@ -22,10 +22,7 @@ class NadMixer(BaseMixer): currently used by this mixer. Sadly, this means that if you use the remote control to change the volume - on the amplifier, Mopidy will no longer report the correct volume. To - recalibrate the mixer, set the volume to 0 through Mopidy. This will reset - the amplifier to a known state, including powering on the device, selecting - the configured speakers and input sources. + on the amplifier, Mopidy will no longer report the correct volume. **Dependencies** @@ -51,8 +48,6 @@ class NadMixer(BaseMixer): def _set_volume(self, volume): self._volume = volume - if volume == 0: - self._pipe.send({'command': 'reset_device'}) self._pipe.send({'command': 'set_volume', 'volume': volume}) From 968433641b68dccff26a66780f70986ec36bce27 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 18:01:16 +0200 Subject: [PATCH 7/8] Add MIXER_MAX_VOLUME setting for capping the max volume --- docs/changes.rst | 1 + mopidy/mixers/__init__.py | 7 +++++-- mopidy/settings.py | 10 ++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index d3ed3db2..12028a17 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -34,6 +34,7 @@ greatly improved MPD client support. ``SPOTIFY_LIB_APPKEY`` setting is thus removed. - Added new :mod:`mopidy.mixers.GStreamerSoftwareMixer` which now is the default mixer on all platforms. +- New setting ``MIXER_MAX_VOLUME`` for capping the maximum output volume. - MPD frontend: - Relocate from :mod:`mopidy.mpd` to :mod:`mopidy.frontends.mpd`. diff --git a/mopidy/mixers/__init__.py b/mopidy/mixers/__init__.py index 3ef1b645..b03e3a4b 100644 --- a/mopidy/mixers/__init__.py +++ b/mopidy/mixers/__init__.py @@ -1,3 +1,5 @@ +from mopidy import settings + class BaseMixer(object): """ :param backend: a backend instance @@ -6,6 +8,7 @@ class BaseMixer(object): def __init__(self, backend, *args, **kwargs): self.backend = backend + self.amplification_factor = settings.MIXER_MAX_VOLUME / 100.0 @property def volume(self): @@ -15,11 +18,11 @@ class BaseMixer(object): Integer in range [0, 100]. :class:`None` if unknown. Values below 0 is equal to 0. Values above 100 is equal to 100. """ - return self._get_volume() + return self._get_volume() / self.amplification_factor @volume.setter def volume(self, volume): - volume = int(volume) + volume = int(volume) * self.amplification_factor if volume < 0: volume = 0 elif volume > 100: diff --git a/mopidy/settings.py b/mopidy/settings.py index 232adda8..67b0c24f 100644 --- a/mopidy/settings.py +++ b/mopidy/settings.py @@ -115,6 +115,16 @@ MIXER_EXT_SPEAKERS_A = None #: Default: :class:`None`. MIXER_EXT_SPEAKERS_B = None +#: The maximum volume. Integer in the range 0 to 100. +#: +#: If this settings is set to 80, the mixer will set the actual volume to 80 +#: when asked to set it to 100. +#: +#: Default:: +#: +#: MIXER_MAX_VOLUME = 100 +MIXER_MAX_VOLUME = 100 + #: Audio output handler to use. #: #: Default:: From 074976d9f3bdff2b42e6fa189ad97a033c60a689 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 14 Aug 2010 18:18:15 +0200 Subject: [PATCH 8/8] Test MIXER_MAX_VOLUME and fix detected bugs --- mopidy/mixers/__init__.py | 6 ++++-- tests/mixers/base_test.py | 14 ++++++-------- tests/mixers/denon_test.py | 7 +++++-- tests/mixers/dummy_test.py | 17 +++++++++++++++++ 4 files changed, 32 insertions(+), 12 deletions(-) create mode 100644 tests/mixers/dummy_test.py diff --git a/mopidy/mixers/__init__.py b/mopidy/mixers/__init__.py index b03e3a4b..c9543863 100644 --- a/mopidy/mixers/__init__.py +++ b/mopidy/mixers/__init__.py @@ -18,11 +18,13 @@ class BaseMixer(object): Integer in range [0, 100]. :class:`None` if unknown. Values below 0 is equal to 0. Values above 100 is equal to 100. """ - return self._get_volume() / self.amplification_factor + if self._get_volume() is None: + return None + return int(self._get_volume() / self.amplification_factor) @volume.setter def volume(self, volume): - volume = int(volume) * self.amplification_factor + volume = int(int(volume) * self.amplification_factor) if volume < 0: volume = 0 elif volume > 100: diff --git a/tests/mixers/base_test.py b/tests/mixers/base_test.py index 292edcda..d6129ad5 100644 --- a/tests/mixers/base_test.py +++ b/tests/mixers/base_test.py @@ -1,18 +1,16 @@ -import unittest - -from mopidy.mixers.dummy import DummyMixer - -class BaseMixerTest(unittest.TestCase): +class BaseMixerTest(object): MIN = 0 MAX = 100 - ACTUAL_MIN = MIN ACTUAL_MAX = MAX - INITIAL = None + mixer_class = None + def setUp(self): - self.mixer = DummyMixer(None) + assert self.mixer_class is not None, \ + "mixer_class must be set in subclass" + self.mixer = self.mixer_class(None) def test_initial_volume(self): self.assertEqual(self.mixer.volume, self.INITIAL) diff --git a/tests/mixers/denon_test.py b/tests/mixers/denon_test.py index e0f59705..5370f155 100644 --- a/tests/mixers/denon_test.py +++ b/tests/mixers/denon_test.py @@ -1,3 +1,5 @@ +import unittest + from mopidy.mixers.denon import DenonMixer from tests.mixers.base_test import BaseMixerTest @@ -22,11 +24,12 @@ class DenonMixerDeviceMock(object): def open(self): self._open = True -class DenonMixerTest(BaseMixerTest): +class DenonMixerTest(BaseMixerTest, unittest.TestCase): ACTUAL_MAX = 99 - INITIAL = 1 + mixer_class = DenonMixer + def setUp(self): self.device = DenonMixerDeviceMock() self.mixer = DenonMixer(None, device=self.device) diff --git a/tests/mixers/dummy_test.py b/tests/mixers/dummy_test.py new file mode 100644 index 00000000..334dc8a1 --- /dev/null +++ b/tests/mixers/dummy_test.py @@ -0,0 +1,17 @@ +import unittest + +from mopidy.mixers.dummy import DummyMixer +from tests.mixers.base_test import BaseMixerTest + +class DenonMixerTest(BaseMixerTest, unittest.TestCase): + mixer_class = DummyMixer + + def test_set_volume_is_capped(self): + self.mixer.amplification_factor = 0.5 + self.mixer.volume = 100 + self.assertEquals(self.mixer._volume, 50) + + def test_get_volume_does_not_show_that_the_volume_is_capped(self): + self.mixer.amplification_factor = 0.5 + self.mixer._volume = 50 + self.assertEquals(self.mixer.volume, 100)