Merge pull request #1176 from tkem/fix/1175-v1.0.x

m3u: Fix encoding error when saving playlists with non-ASCII track titles

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)
 ===================

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)

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)

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')