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. diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index a4a987a554..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 @@ -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()) @@ -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 @@ -454,7 +459,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.""" @@ -577,23 +583,27 @@ 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): - return self.cmd_playlistinfo(conn, self._id_to_index(track_id)) + 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) def cmd_plchanges(self, conn, version): """Sends playlist changes since the given version. @@ -624,7 +634,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) @@ -645,7 +654,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: @@ -844,6 +852,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. @@ -1094,6 +1105,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() @@ -1111,19 +1124,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)) @@ -1131,10 +1135,27 @@ 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 + 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 @@ -1335,18 +1356,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): @@ -1543,7 +1570,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) diff --git a/docs/changelog.rst b/docs/changelog.rst index 7bfe10177f..e583254821 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: diff --git a/docs/plugins/bpd.rst b/docs/plugins/bpd.rst index 87c931793a..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 @@ -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 66f1f5d38d..959d77eb3c 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]) @@ -362,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: @@ -392,8 +393,31 @@ def test_empty_request(self): class BPDQueryTest(BPDTestHelper): test_implements_query = implements({ - 'clearerror', 'currentsong', 'stats', - }) + '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: @@ -410,10 +434,19 @@ 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())) + 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): @@ -449,6 +482,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({ @@ -615,8 +656,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: @@ -630,9 +676,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 +693,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) @@ -684,24 +768,48 @@ 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', }, 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) 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: + 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) + 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): @@ -831,8 +939,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: @@ -852,19 +976,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):