From ea5dff109ec8a8ea1d3b3dd086a2b8242bdf0bcb Mon Sep 17 00:00:00 2001 From: Thomas Kemmer Date: Sun, 10 May 2015 20:57:39 +0200 Subject: [PATCH] m3u: Fix encoding error when saving playlists with non-ASCII track titles. --- docs/changelog.rst | 3 +++ mopidy/m3u/playlists.py | 16 +--------------- mopidy/m3u/translator.py | 16 ++++++++++++++++ tests/m3u/test_playlists.py | 23 +++++++++++++++++++++-- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 00cee7b8..d87b16a9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,6 +13,9 @@ Bug fix release. - Core: Add workaround for playlist providers that do not support creating playlists. (Fixes: :issue:`1162`, PR :issue:`1165`) +- M3U: Fix encoding error when saving playlists with non-ASCII track + titles. (Fixes: :issue:`1175`, PR :issue:`1176`) + v1.0.4 (2015-04-30) =================== diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index c09eccdf..8800e468 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -88,11 +88,6 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): self._playlists[playlist.uri] = playlist return playlist - def _write_m3u_extinf(self, file_handle, track): - title = track.name.encode('latin-1', 'replace') - runtime = track.length // 1000 if track.length else -1 - file_handle.write('#EXTINF:' + str(runtime) + ',' + title + '\n') - def _sanitize_m3u_name(self, name, encoding=sys.getfilesystemencoding()): name = self._invalid_filename_chars.sub('|', name.strip()) # make sure we end up with a valid path segment @@ -113,15 +108,6 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): name, _ = os.path.splitext(os.path.basename(path).decode(encoding)) else: raise ValueError('M3U playlist needs name or URI') - extended = any(track.name for track in playlist.tracks) - - with open(path, 'w') as file_handle: - if extended: - file_handle.write('#EXTM3U\n') - for track in playlist.tracks: - if extended and track.name: - self._write_m3u_extinf(file_handle, track) - file_handle.write(track.uri + '\n') - + translator.save_m3u(path, playlist.tracks, 'latin1') # assert playlist name matches file name/uri return playlist.copy(uri=uri, name=name) diff --git a/mopidy/m3u/translator.py b/mopidy/m3u/translator.py index 4eefce9d..a6e006b1 100644 --- a/mopidy/m3u/translator.py +++ b/mopidy/m3u/translator.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +import codecs import logging import os import re @@ -108,3 +109,18 @@ def parse_m3u(file_path, media_dir=None): track = Track() return tracks + + +def save_m3u(filename, tracks, encoding='latin1', errors='replace'): + extended = any(track.name for track in tracks) + # codecs.open() always uses binary mode, just being explicit here + with codecs.open(filename, 'wb', encoding, errors) as m3u: + if extended: + m3u.write('#EXTM3U' + os.linesep) + for track in tracks: + if extended and track.name: + m3u.write('#EXTINF:%d,%s%s' % ( + track.length // 1000 if track.length else -1, + track.name, + os.linesep)) + m3u.write(track.uri + os.linesep) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 355aabf5..b7ac827f 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -107,9 +107,28 @@ class M3UPlaylistsProviderTest(unittest.TestCase): path = playlist_uri_to_path(playlist.uri, self.playlists_dir) with open(path) as f: - contents = f.read().splitlines() + m3u = f.read().splitlines() + self.assertEqual(['#EXTM3U', '#EXTINF:60,Test', track.uri], m3u) - self.assertEqual(contents, ['#EXTM3U', '#EXTINF:60,Test', track.uri]) + def test_latin1_playlist_contents_is_written_to_disk(self): + track = Track(uri=generate_song(1), name='Test\x9f', length=60000) + playlist = self.core.playlists.create('test') + playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + + with open(path, 'rb') as f: + m3u = f.read().splitlines() + self.assertEqual([b'#EXTM3U', b'#EXTINF:60,Test\x9f', track.uri], m3u) + + def test_utf8_playlist_contents_is_replaced_and_written_to_disk(self): + track = Track(uri=generate_song(1), name='Test\u07b4', length=60000) + playlist = self.core.playlists.create('test') + playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + path = playlist_uri_to_path(playlist.uri, self.playlists_dir) + + with open(path, 'rb') as f: + m3u = f.read().splitlines() + self.assertEqual([b'#EXTM3U', b'#EXTINF:60,Test?', track.uri], m3u) def test_playlists_are_loaded_at_startup(self): track = Track(uri='dummy:track:path2')