From 80286ea89871ef0313230055c6c80e945b757cac Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 14:18:31 +1000 Subject: [PATCH 01/14] bluelet: catch ECONNRESET When ncmpcpp quits after an error it causes a "connection reset by peer" exception, also known as ECONNRESET (104) in errno terms. In Python 2 this is mapped to a `socket.error` and in Python 3 this is `ConnectionResetError` which is thankfully a subclass of the `socket.error` exception class. --- beets/util/bluelet.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beets/util/bluelet.py b/beets/util/bluelet.py index 0da17559b9..dcc80e041e 100644 --- a/beets/util/bluelet.py +++ b/beets/util/bluelet.py @@ -346,6 +346,10 @@ def kill_thread(coro): exc.args[0] == errno.EPIPE: # Broken pipe. Remote host disconnected. pass + elif isinstance(exc.args, tuple) and \ + exc.args[0] == errno.ECONNRESET: + # Connection was reset by peer. + pass else: traceback.print_exc() # Abort the coroutine. From 64ed54330bad3d9dc5978f793e2a03ffb0338b1a Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Fri, 12 Apr 2019 12:23:07 +1000 Subject: [PATCH 02/14] bpd: mention control socket address in log --- beetsplug/bpd/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index a4a987a554..1087f80373 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1094,6 +1094,8 @@ def __init__(self, library, host, port, password, ctrl_port, log): self.cmd_update(None) log.info(u'Server ready and listening on {}:{}'.format( host, port)) + log.debug(u'Listening for control signals on {}:{}'.format( + host, ctrl_port)) def run(self): self.player.run() From 59c506990a386fc1ec61e30b6f7e0d41aaf3f92b Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 14:16:43 +1000 Subject: [PATCH 03/14] bpd: fix bug in playlistid The playlistid command is supposed to list the whole playlist if no argument is provided, but we were accidentally trying to look up an impossible negative id in that case causing an error to always be returned. --- beetsplug/bpd/__init__.py | 4 +++- test/test_player.py | 12 +++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 1087f80373..5bf35e1d59 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -593,7 +593,9 @@ def cmd_playlistinfo(self, conn, index=-1): yield self._item_info(track) def cmd_playlistid(self, conn, track_id=-1): - return self.cmd_playlistinfo(conn, self._id_to_index(track_id)) + if track_id != -1: + track_id = self._id_to_index(track_id) + return self.cmd_playlistinfo(conn, track_id) def cmd_plchanges(self, conn, version): """Sends playlist changes since the given version. diff --git a/test/test_player.py b/test/test_player.py index 66f1f5d38d..bce1a06250 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -684,7 +684,7 @@ def test_cmd_previous(self): class BPDQueueTest(BPDTestHelper): test_implements_queue = implements({ 'addid', 'clear', 'delete', 'deleteid', 'move', - 'moveid', 'playlist', 'playlistfind', 'playlistid', + 'moveid', 'playlist', 'playlistfind', 'playlistsearch', 'plchanges', 'plchangesposid', 'prio', 'prioid', 'rangeid', 'shuffle', 'swap', 'swapid', 'addtagid', 'cleartagid', @@ -703,6 +703,16 @@ def test_cmd_playlistinfo(self): ('playlistinfo', '200')) self._assert_failed(responses, bpd.ERROR_ARG, pos=2) + def test_cmd_playlistid(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1, self.item2) + responses = client.send_commands( + ('playlistid', '2'), + ('playlistid',)) + self._assert_ok(*responses) + self.assertEqual('Track Two Title', responses[0].data['Title']) + self.assertEqual(['1', '2'], responses[1].data['Track']) + class BPDPlaylistsTest(BPDTestHelper): test_implements_playlists = implements({'playlistadd'}) From 5c37a58ad6cf4c8f631184df86419077d376b6f4 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 14:18:03 +1000 Subject: [PATCH 04/14] bpd: add more tests --- test/test_player.py | 54 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/test/test_player.py b/test/test_player.py index bce1a06250..b4ed03618a 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -392,7 +392,7 @@ def test_empty_request(self): class BPDQueryTest(BPDTestHelper): test_implements_query = implements({ - 'clearerror', 'currentsong', 'stats', + 'clearerror', 'currentsong', }) def test_cmd_status(self): @@ -414,6 +414,14 @@ def test_cmd_status(self): } self.assertEqual(fields_playing, set(responses[2].data.keys())) + def test_cmd_stats(self): + with self.run_bpd() as client: + response = client.send_command('stats') + self._assert_ok(response) + details = {'artists', 'albums', 'songs', 'uptime', 'db_playtime', + 'db_update', 'playtime'} + self.assertEqual(details, set(response.data.keys())) + def test_cmd_idle(self): def _toggle(c): for _ in range(3): @@ -630,9 +638,8 @@ def test_cmd_replay_gain(self): class BPDControlTest(BPDTestHelper): test_implements_control = implements({ - 'pause', 'playid', 'seek', - 'seekid', 'seekcur', 'stop', - }, expectedFailure=True) + 'seek', 'seekid', 'seekcur', + }, expectedFailure=True) def test_cmd_play(self): with self.run_bpd() as client: @@ -648,6 +655,45 @@ def test_cmd_play(self): self.assertEqual('play', responses[2].data['state']) self.assertEqual('2', responses[4].data['Id']) + def test_cmd_playid(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1, self.item2) + responses = client.send_commands( + ('playid', '2'), + ('currentsong',), + ('clear',)) + self._bpd_add(client, self.item2, self.item1) + responses.extend(client.send_commands( + ('playid', '2'), + ('currentsong',))) + self._assert_ok(*responses) + self.assertEqual('2', responses[1].data['Id']) + self.assertEqual('2', responses[4].data['Id']) + + def test_cmd_pause(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1) + responses = client.send_commands( + ('play',), + ('pause',), + ('status',), + ('currentsong',)) + self._assert_ok(*responses) + self.assertEqual('pause', responses[2].data['state']) + self.assertEqual('1', responses[3].data['Id']) + + def test_cmd_stop(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1) + responses = client.send_commands( + ('play',), + ('stop',), + ('status',), + ('currentsong',)) + self._assert_ok(*responses) + self.assertEqual('stop', responses[2].data['state']) + self.assertNotIn('Id', responses[3].data) + def test_cmd_next(self): with self.run_bpd() as client: self._bpd_add(client, self.item1, self.item2) From 1a5263b68ff193941ec8d7af941fa8a94ad7e5a4 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Mon, 15 Apr 2019 14:19:09 +1000 Subject: [PATCH 05/14] bpd: support volume command for real --- beetsplug/bpd/__init__.py | 3 ++- test/test_player.py | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 5bf35e1d59..05bde6d331 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -454,7 +454,8 @@ def cmd_setvol(self, conn, vol): def cmd_volume(self, conn, vol_delta): """Deprecated command to change the volume by a relative amount.""" - raise BPDError(ERROR_SYSTEM, u'No mixer') + vol_delta = cast_arg(int, vol_delta) + return self.cmd_setvol(conn, self.volume + vol_delta) def cmd_crossfade(self, conn, crossfade): """Set the number of seconds of crossfading.""" diff --git a/test/test_player.py b/test/test_player.py index b4ed03618a..048f2ceddd 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -623,8 +623,13 @@ def test_cmd_setvol(self): def test_cmd_volume(self): with self.run_bpd() as client: - response = client.send_command('volume', '10') - self._assert_failed(response, bpd.ERROR_SYSTEM) + responses = client.send_commands( + ('setvol', '10'), + ('volume', '5'), + ('volume', '-2'), + ('status',)) + self._assert_ok(*responses) + self.assertEqual('13', responses[3].data['volume']) def test_cmd_replay_gain(self): with self.run_bpd() as client: From e708d28f850893f1aa529d23a2172d30f6dee3be Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Fri, 19 Apr 2019 12:59:46 +1000 Subject: [PATCH 06/14] bpd: allow fractional seconds in seek The documented type is float, not integer, and clients like mpDris2 send fractional seconds, causing them to crash if these values ar enot accepted. --- beetsplug/bpd/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 05bde6d331..7c40cb6b6b 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1548,7 +1548,7 @@ def cmd_stop(self, conn): def cmd_seek(self, conn, index, pos): """Seeks to the specified position in the specified song.""" index = cast_arg(int, index) - pos = cast_arg(int, pos) + pos = cast_arg(float, pos) super(Server, self).cmd_seek(conn, index, pos) self.player.seek(pos) From 27c462d287a7c4788a2f3aab6fe9410ea19098c2 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Mon, 15 Apr 2019 15:08:47 +1000 Subject: [PATCH 07/14] bpd: make noidle a no-op outside idle mode The real MPD ignores `noidle` when the client is not idle. It doesn't even send a successful response, just ignores the command. Although I don't understand why a client would fail to keep track of its own state, it seems that this is necessary to get ncmpcpp working. --- beetsplug/bpd/__init__.py | 3 +++ test/test_player.py | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 7c40cb6b6b..97d89d9140 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -847,6 +847,9 @@ def run(self): yield self.send(err.response()) break continue + if line == u'noidle': + # When not in idle, this command sends no response. + continue if clist is not None: # Command list already opened. diff --git a/test/test_player.py b/test/test_player.py index 048f2ceddd..f22c19261a 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -457,6 +457,14 @@ def test_cmd_noidle(self): response = client.send_command('noidle') self._assert_ok(response) + def test_cmd_noidle_when_not_idle(self): + with self.run_bpd() as client: + # Manually send a command without reading a response. + request = client.serialise_command('noidle') + client.sock.sendall(request) + response = client.send_command('notacommand') + self._assert_failed(response, bpd.ERROR_UNKNOWN) + class BPDPlaybackTest(BPDTestHelper): test_implements_playback = implements({ From 5187100294a9b36581d358c755ff39051c956fc6 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 16:23:42 +1000 Subject: [PATCH 08/14] bpd: accept all idle events --- beetsplug/bpd/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 97d89d9140..487999cc71 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -77,8 +77,8 @@ SUBSYSTEMS = [ u'update', u'player', u'mixer', u'options', u'playlist', u'database', # Related to unsupported commands: - # u'stored_playlist', u'output', u'subscription', u'sticker', u'message', - # u'partition', + u'stored_playlist', u'output', u'subscription', u'sticker', u'message', + u'partition', ] ITEM_KEYS_WRITABLE = set(MediaFile.fields()).intersection(Item._fields.keys()) From cc2c35221d28b9be80eb4b1be5c40fc27e0cfd5c Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Thu, 25 Apr 2019 11:41:00 +1000 Subject: [PATCH 09/14] bpd: avoid sending playlist events on navigation --- beetsplug/bpd/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 487999cc71..dca3310d80 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -627,7 +627,6 @@ def cmd_next(self, conn): """Advance to the next song in the playlist.""" old_index = self.current_index self.current_index = self._succ_idx() - self._send_event('playlist') if self.consume: # TODO how does consume interact with single+repeat? self.playlist.pop(old_index) @@ -648,7 +647,6 @@ def cmd_previous(self, conn): """Step back to the last song.""" old_index = self.current_index self.current_index = self._prev_idx() - self._send_event('playlist') if self.consume: self.playlist.pop(old_index) if self.current_index < 0: From fdd809fd36def07d01b770e7ce04855a1cfb8f77 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Thu, 25 Apr 2019 13:08:26 +1000 Subject: [PATCH 10/14] bpd: support more tagtypes --- beetsplug/bpd/__init__.py | 29 ++++++++-------- docs/plugins/bpd.rst | 3 +- test/test_player.py | 70 +++++++++++++++++++++++++++++++-------- 3 files changed, 72 insertions(+), 30 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index dca3310d80..95598ff18d 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1117,19 +1117,10 @@ def _item_info(self, item): info_lines = [ u'file: ' + item.destination(fragment=True), u'Time: ' + six.text_type(int(item.length)), - u'Title: ' + item.title, - u'Artist: ' + item.artist, - u'Album: ' + item.album, - u'Genre: ' + item.genre, + u'duration: ' + u'{:.3f}'.format(item.length), + u'Id: ' + six.text_type(item.id), ] - track = six.text_type(item.track) - if item.tracktotal: - track += u'/' + six.text_type(item.tracktotal) - info_lines.append(u'Track: ' + track) - - info_lines.append(u'Date: ' + six.text_type(item.year)) - try: pos = self._id_to_index(item.id) info_lines.append(u'Pos: ' + six.text_type(pos)) @@ -1137,7 +1128,9 @@ def _item_info(self, item): # Don't include position if not in playlist. pass - info_lines.append(u'Id: ' + six.text_type(item.id)) + for tagtype, field in self.tagtype_map.items(): + info_lines.append(u'{}: {}'.format( + tagtype, six.text_type(getattr(item, field)))) return info_lines @@ -1341,18 +1334,24 @@ def cmd_decoders(self, conn): tagtype_map = { u'Artist': u'artist', + u'ArtistSort': u'artist_sort', u'Album': u'album', u'Title': u'title', u'Track': u'track', u'AlbumArtist': u'albumartist', u'AlbumArtistSort': u'albumartist_sort', - # Name? + u'Label': u'label', u'Genre': u'genre', u'Date': u'year', + u'OriginalDate': u'original_year', u'Composer': u'composer', - # Performer? u'Disc': u'disc', - u'filename': u'path', # Suspect. + u'Comment': u'comments', + u'MUSICBRAINZ_TRACKID': u'mb_trackid', + u'MUSICBRAINZ_ALBUMID': u'mb_albumid', + u'MUSICBRAINZ_ARTISTID': u'mb_artistid', + u'MUSICBRAINZ_ALBUMARTISTID': u'mb_albumartistid', + u'MUSICBRAINZ_RELEASETRACKID': u'mb_releasetrackid', } def cmd_tagtypes(self, conn): diff --git a/docs/plugins/bpd.rst b/docs/plugins/bpd.rst index 87c931793a..8bfe456ad7 100644 --- a/docs/plugins/bpd.rst +++ b/docs/plugins/bpd.rst @@ -125,8 +125,7 @@ These are some of the known differences between BPD and MPD: * Advanced playback features like cross-fade, ReplayGain and MixRamp are not supported due to BPD's simple audio player backend. * Advanced query syntax is not currently supported. -* Not all tags (fields) are currently exposed to BPD. Clients also can't use - the ``tagtypes`` mask to hide fields. +* Clients can't use the ``tagtypes`` mask to hide fields. * BPD's ``random`` mode is not deterministic and doesn't support priorities. * Mounts and streams are not supported. BPD can only play files from disk. * Stickers are not supported (although this is basically a flexattr in beets diff --git a/test/test_player.py b/test/test_player.py index f22c19261a..874a2db525 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -392,8 +392,31 @@ def test_empty_request(self): class BPDQueryTest(BPDTestHelper): test_implements_query = implements({ - 'clearerror', 'currentsong', - }) + 'clearerror', + }) + + def test_cmd_currentsong(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1) + responses = client.send_commands( + ('play',), + ('currentsong',), + ('stop',), + ('currentsong',)) + self._assert_ok(*responses) + self.assertEqual('1', responses[1].data['Id']) + self.assertNotIn('Id', responses[3].data) + + def test_cmd_currentsong_tagtypes(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1) + responses = client.send_commands( + ('play',), + ('currentsong',)) + self._assert_ok(*responses) + self.assertEqual( + BPDConnectionTest.TAGTYPES.union(BPDQueueTest.METADATA), + set(responses[1].data.keys())) def test_cmd_status(self): with self.run_bpd() as client: @@ -749,6 +772,8 @@ class BPDQueueTest(BPDTestHelper): 'swap', 'swapid', 'addtagid', 'cleartagid', }, expectedFailure=True) + METADATA = {'Pos', 'Time', 'Id', 'file', 'duration'} + def test_cmd_add(self): with self.run_bpd() as client: self._bpd_add(client, self.item1) @@ -762,6 +787,15 @@ def test_cmd_playlistinfo(self): ('playlistinfo', '200')) self._assert_failed(responses, bpd.ERROR_ARG, pos=2) + def test_cmd_playlistinfo_tagtypes(self): + with self.run_bpd() as client: + self._bpd_add(client, self.item1) + response = client.send_command('playlistinfo', '0') + self._assert_ok(response) + self.assertEqual( + BPDConnectionTest.TAGTYPES.union(BPDQueueTest.METADATA), + set(response.data.keys())) + def test_cmd_playlistid(self): with self.run_bpd() as client: self._bpd_add(client, self.item1, self.item2) @@ -900,8 +934,24 @@ class BPDStickerTest(BPDTestHelper): class BPDConnectionTest(BPDTestHelper): test_implements_connection = implements({ - 'close', 'kill', 'tagtypes', - }) + 'close', 'kill', + }) + + ALL_MPD_TAGTYPES = { + 'Artist', 'ArtistSort', 'Album', 'AlbumSort', 'AlbumArtist', + 'AlbumArtistSort', 'Title', 'Track', 'Name', 'Genre', 'Date', + 'Composer', 'Performer', 'Comment', 'Disc', 'Label', + 'OriginalDate', 'MUSICBRAINZ_ARTISTID', 'MUSICBRAINZ_ALBUMID', + 'MUSICBRAINZ_ALBUMARTISTID', 'MUSICBRAINZ_TRACKID', + 'MUSICBRAINZ_RELEASETRACKID', 'MUSICBRAINZ_WORKID', + } + UNSUPPORTED_TAGTYPES = { + 'MUSICBRAINZ_WORKID', # not tracked by beets + 'Performer', # not tracked by beets + 'AlbumSort', # not tracked by beets + 'Name', # junk field for internet radio + } + TAGTYPES = ALL_MPD_TAGTYPES.difference(UNSUPPORTED_TAGTYPES) def test_cmd_password(self): with self.run_bpd(password='abc123') as client: @@ -921,19 +971,13 @@ def test_cmd_ping(self): response = client.send_command('ping') self._assert_ok(response) - @unittest.skip def test_cmd_tagtypes(self): with self.run_bpd() as client: response = client.send_command('tagtypes') self._assert_ok(response) - self.assertEqual({ - 'Artist', 'ArtistSort', 'Album', 'AlbumSort', 'AlbumArtist', - 'AlbumArtistSort', 'Title', 'Track', 'Name', 'Genre', 'Date', - 'Composer', 'Performer', 'Comment', 'Disc', 'Label', - 'OriginalDate', 'MUSICBRAINZ_ARTISTID', 'MUSICBRAINZ_ALBUMID', - 'MUSICBRAINZ_ALBUMARTISTID', 'MUSICBRAINZ_TRACKID', - 'MUSICBRAINZ_RELEASETRACKID', 'MUSICBRAINZ_WORKID', - }, set(response.data['tag'])) + self.assertEqual( + self.TAGTYPES, + set(response.data['tagtype'])) @unittest.skip def test_tagtypes_mask(self): From dc7e3b9b6a124ff7b1df31683f00aecfc3ef15bd Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Fri, 12 Apr 2019 12:30:40 +1000 Subject: [PATCH 11/14] bpd: support nextsong in status --- beetsplug/bpd/__init__.py | 5 +++++ test/test_player.py | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 95598ff18d..8d90b5cf6e 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -412,6 +412,11 @@ def cmd_status(self, conn): current_id = self._item_id(self.playlist[self.current_index]) yield u'song: ' + six.text_type(self.current_index) yield u'songid: ' + six.text_type(current_id) + if len(self.playlist) > self.current_index + 1: + # If there's a next song, report its index too. + next_id = self._item_id(self.playlist[self.current_index + 1]) + yield u'nextsong: ' + six.text_type(self.current_index + 1) + yield u'nextsongid: ' + six.text_type(next_id) if self.error: yield u'error: ' + self.error diff --git a/test/test_player.py b/test/test_player.py index 874a2db525..9bd75519fd 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -433,7 +433,8 @@ def test_cmd_status(self): } self.assertEqual(fields_not_playing, set(responses[0].data.keys())) fields_playing = fields_not_playing | { - 'song', 'songid', 'time', 'elapsed', 'bitrate', 'duration', 'audio' + 'song', 'songid', 'time', 'elapsed', 'bitrate', 'duration', + 'audio', 'nextsong', 'nextsongid' } self.assertEqual(fields_playing, set(responses[2].data.keys())) From d8be83bc0d504744f6ce699b01040a7465caf1b6 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 15:28:53 +1000 Subject: [PATCH 12/14] bpd: support ranges in playlistid --- beetsplug/bpd/__init__.py | 31 ++++++++++++++++++++++++------- test/test_player.py | 10 +++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 8d90b5cf6e..ab515b3888 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -583,23 +583,25 @@ def cmd_urlhandlers(self, conn): """Indicates supported URL schemes. None by default.""" pass - def cmd_playlistinfo(self, conn, index=-1): + def cmd_playlistinfo(self, conn, index=None): """Gives metadata information about the entire playlist or a single track, given by its index. """ - index = cast_arg(int, index) - if index == -1: + if index is None: for track in self.playlist: yield self._item_info(track) else: + indices = self._parse_range(index, accept_single_number=True) try: - track = self.playlist[index] + tracks = [self.playlist[i] for i in indices] except IndexError: raise ArgumentIndexError() - yield self._item_info(track) + for track in tracks: + yield self._item_info(track) - def cmd_playlistid(self, conn, track_id=-1): - if track_id != -1: + def cmd_playlistid(self, conn, track_id=None): + if track_id is not None: + track_id = cast_arg(int, track_id) track_id = self._id_to_index(track_id) return self.cmd_playlistinfo(conn, track_id) @@ -1139,6 +1141,21 @@ def _item_info(self, item): return info_lines + def _parse_range(self, items, accept_single_number=False): + """Convert a range of positions to a list of item info. + MPD specifies ranges as START:STOP (endpoint excluded) for some + commands. Sometimes a single number can be provided instead. + """ + try: + start, stop = str(items).split(':', 1) + except ValueError: + if accept_single_number: + return [cast_arg(int, items)] + raise BPDError(ERROR_ARG, u'bad range syntax') + start = cast_arg(int, start) + stop = cast_arg(int, stop) + return range(start, stop) + def _item_id(self, item): return item.id diff --git a/test/test_player.py b/test/test_player.py index 9bd75519fd..735f9c62c6 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -339,8 +339,9 @@ def _assert_failed(self, response, code, pos=None): previous_commands = response[0:pos] self._assert_ok(*previous_commands) response = response[pos] - self.assertEqual(pos, response.err_data[1]) self.assertFalse(response.ok) + if pos is not None: + self.assertEqual(pos, response.err_data[1]) if code is not None: self.assertEqual(code, response.err_data[0]) @@ -781,12 +782,15 @@ def test_cmd_add(self): def test_cmd_playlistinfo(self): with self.run_bpd() as client: - self._bpd_add(client, self.item1) + self._bpd_add(client, self.item1, self.item2) responses = client.send_commands( ('playlistinfo',), ('playlistinfo', '0'), + ('playlistinfo', '0:2'), ('playlistinfo', '200')) - self._assert_failed(responses, bpd.ERROR_ARG, pos=2) + self._assert_failed(responses, bpd.ERROR_ARG, pos=3) + self.assertEqual('1', responses[1].data['Id']) + self.assertEqual(['1', '2'], responses[2].data['Id']) def test_cmd_playlistinfo_tagtypes(self): with self.run_bpd() as client: From 62aa358ce7fa8f6a0193009d403ca3936d3df890 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sat, 13 Apr 2019 14:18:54 +1000 Subject: [PATCH 13/14] bpd: bump protocol version to 0.16 --- beetsplug/bpd/__init__.py | 2 +- docs/plugins/bpd.rst | 2 +- test/test_player.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index ab515b3888..045bce0356 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -39,7 +39,7 @@ from mediafile import MediaFile import six -PROTOCOL_VERSION = '0.14.0' +PROTOCOL_VERSION = '0.16.0' BUFSIZE = 1024 HELLO = u'OK MPD %s' % PROTOCOL_VERSION diff --git a/docs/plugins/bpd.rst b/docs/plugins/bpd.rst index 8bfe456ad7..c1a94e972f 100644 --- a/docs/plugins/bpd.rst +++ b/docs/plugins/bpd.rst @@ -103,7 +103,7 @@ but doesn't support many advanced playback features. Differences from the real MPD ----------------------------- -BPD currently supports version 0.14 of `the MPD protocol`_, but several of the +BPD currently supports version 0.16 of `the MPD protocol`_, but several of the commands and features are "pretend" implementations or have slightly different behaviour to their MPD equivalents. BPD aims to look enough like MPD that it can interact with the ecosystem of clients, but doesn't try to be diff --git a/test/test_player.py b/test/test_player.py index 735f9c62c6..959d77eb3c 100644 --- a/test/test_player.py +++ b/test/test_player.py @@ -363,7 +363,7 @@ def _bpd_add(self, client, *items, **kwargs): class BPDTest(BPDTestHelper): def test_server_hello(self): with self.run_bpd(do_hello=False) as client: - self.assertEqual(client.readline(), b'OK MPD 0.14.0\n') + self.assertEqual(client.readline(), b'OK MPD 0.16.0\n') def test_unknown_cmd(self): with self.run_bpd() as client: From 65432bbb2d82cb402478dea77b2f1199c5ef41d8 Mon Sep 17 00:00:00 2001 From: Carl Suster Date: Sun, 2 Jun 2019 23:50:20 +1000 Subject: [PATCH 14/14] Changelog for #3214 --- docs/changelog.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 7a74f556b8..9e2bfd8adf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,12 @@ New features: (the MBID), and ``work_disambig`` (the disambiguation string). Thanks to :user:`dosoe`. :bug:`2580` :bug:`3272` +* :doc:`/plugins/bpd`: BPD now supports most of the features of version 0.16 + of the MPD protocol. This is enough to get it talking to more complicated + clients like ncmpcpp, but there are still some incompatibilities, largely due + to MPD commands we don't support yet. Let us know if you find an MPD client + that doesn't get along with BPD! + :bug:`3214` :bug:`800` Fixes: @@ -20,6 +26,10 @@ Fixes: objects could seem to reuse values from earlier objects when they were missing a value for a given field. These values are now properly undefined. :bug:`2406` +* :doc:`/plugins/bpd`: Seeking by fractions of a second now works as intended, + fixing crashes in MPD clients like mpDris2 on seek. + The ``playlistid`` command now works properly in its zero-argument form. + :bug:`3214` For plugin developers: