convert: New feature "Write m3u playlist to destination folder"

[JOJ0]

Description

• Enables the functionality proposed in Exporting smart playlists for use on other computer #4373
• The convert plugin is suitable to be used as a media file exporter already.
• Thus it makes sense to add a feature to save a playlist file into the export folder containing all exported media files in the order they were exported/converted.
• Since with the convert plugin all sorts of "beets queries" could be exported/converted, the feature uses a more general approach than described in the initial proposal (above linked discussion). To achieve exactly what's described in the discussion, additionally the beets playlist feature can be used.
• Relative paths should be used to make the playlist file portable, e.g used on another computer.
• The classic m3u format does not do well with special characters in media file names. Since the beets "path" variables could be used to write all sorts of characters, the m3u8 format which requires unicode for all media file entries, is required.

To Do

• Documentation. (If you've add a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
• Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
• Tests.
  • convert plugin tests.
  • M3UFile class tests (macOS/Unix/General).
  • M3UFile class tests (Windows).
• Currently there is an issue with VLC media player not reading the generated files correctly. It seems to still have a problem with special characters (e.g square brackets: []). -> Fixed, it was a bug in VLC 3.0.16, it works with 3.0.17.3, see https://forum.videolan.org/viewtopic.php?f=2&t=158920&p=529284#p529284
• It seems that there is another bug (?) in the current VLC version 3.0.17.3 that files containing the # character are not opening, also see same post: https://forum.videolan.org/viewtopic.php?f=2&t=158920&p=529284#p529284
  I'm not considering this a problem with my implementation here. Let's see what happens on that forum post, but for now I check this task as done.
• Test correct reading of the files in other software. So far working fine (all tested on macOS 10.15.3 Catalina):
  • iTunes (latest on Catalina)
  • Traktor Scratch Pro 2 (latest, 2.11.3.17)
  • VLC (3.0.17.3)
• Ensure cross-platform compatibilityEnsure compatibility of playlists generated on an OS for an OS. For example, lists generated on Windows should work on Windows.

[JOJ0]

Usage examples

Simple example

Using a beets query to save a playlist of all files of an artist sorted chronologically:

plugins:
    - convert

convert:
    dest: /Users/jojo/Music/Techno/0-export
    never_convert_lossy_files: yes
    no_convert: format:AAC, format:WMA, format:ALAC, format:FLAC
    threads: 1
    paths:
        default: "$artist - $album - $title [$catalognum] ($year)"
        singleton: "$artist - $album - $title [$catalognum] ($year)"
        comp: "$artist - $album - $title [$catalognum] ($year)"

beet convert -y "artist:Amon Tobin" year+ --dest ~/Downloads/tmp --playlist 0-amon_tobin_chronologically.m3u

More advanced example

An existing playlist (eg. generated with the "smartplaylist" plugin) should be exported to the export folder:

Required config:

plugins:
    - convert
    - playlist

playlist:
    playlist_dir: ~/Music/Techno/0-playlists
convert:
    dest: /Users/jojo/Music/Techno/0-export
    never_convert_lossy_files: yes
    no_convert: format:AAC, format:WMA, format:ALAC, format:FLAC
    threads: 1
    paths:
        default: "$artist - $album - $title [$catalognum] ($year)"
        singleton: "$artist - $album - $title [$catalognum] ($year)"
        comp: "$artist - $album - $title [$catalognum] ($year)"

beet convert -y playlist:/Users/jojo/Music/Techno/0-playlists/110..130_TechnoJungleDub_Lo.m3u --dest ~/Downloads/tmp --playlist 0-play.m3u8

Certainly any playlist file could be exported, no matter if it was created with a beets plugin or manually, as long as it's using media files existing in the beets db already.

It's intended that the playlist file is saved alongside the media files, in this case the path provided by the --dest option. If --dest is not provided with the command it's taken from the config file (as the convert plugin handles it already).

[sampsyo]

Seems like a neat feature! I have a couple of high-level questions about the way the playlist file is generated here:

• Are races possible (when the plugin is running in parallel)? I notice that the playlist is not written out in a single "batch"; rather, the file is re-opened to write every line (to record every file). When the files are being converted in parallel, in separate threads, could this cause a problem where different threads are trying to write to the same file at the same time?
• Is there any way that we could centralize logic with the playlist and/or smartplaylist plugins, which also write .m3u files? Having a single implementation of this could really help ease a maintenance burden, especially if we ever change how we want to write these files to include the extension comment, for example (see Play plugin: allow generation of playlist with M3U extensions (comments) #4356).

[JOJ0]

yes there will most probably be an issue with threading I guess. Actually I wanted to ask you for advice on that topic :-) Thanks for the early review I'll think about your ideas and will hopefully get back to this next week and probably have more questions.

[sampsyo]

Sounds good! I wonder if there's a way to "batch up" all the new paths and then write them out all at once…

[JOJ0]

This is stalling.

There is summer/vacations/other projects need finishing but most of all because I don't get the unittest to work.

Anyway, I have 5 minutes and ask for help. I'm currently trying to write a unittest that checks whether the playlist.m3u8 file was written and is existing. I'm feeling bad already for asking Adrian all the time, he probably has a lot on the plate with this project already. @arsaboo I'm not sure if you're officially a maintainer of the project but you seem highly motivated lately and since you have quite some experience with your Spotify and Plex stuff going on you might be able to help: I'm trying to check if a file was created, but the file doesn't seem to be there: 147e018
This is what I get:

(beets) ➜  beets git:(convert_playlist) python -m unittest test.test_convert.ConvertCliTest.test_playlist
Sending event: database_change
Sending event: database_change
Sending event: item_copied
Sending event: database_change
Sending event: database_change
Sending event: database_change
Sending event: database_change
Sending event: database_change
no user configuration found at /var/folders/3t/l575q0f96fx91pp61zb158z00000gn/T/tmpettdj4j2/config.yaml
data directory: /var/folders/3t/l575q0f96fx91pp61zb158z00000gn/T/tmpettdj4j2
plugin paths:
Sending event: pluginload
the artist - älbum - tïtle 0
Convert? (Y/n) convert: Creating playlist file: /var/folders/3t/l575q0f96fx91pp61zb158z00000gn/T/tmpettdj4j2/convert_dest/playlist.m3u8
E
======================================================================
ERROR: test_playlist (test.test_convert.ConvertCliTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jojo/git/beets/test/test_convert.py", line 254, in test_playlist
    self.run_convert('--playlist', 'playlist.m3u8')
  File "/Users/jojo/git/beets/test/test_convert.py", line 156, in run_convert
    return self.run_convert_path(self.item.path, *args)
  File "/Users/jojo/git/beets/test/test_convert.py", line 152, in run_convert_path
    return self.run_command('convert', *args)
  File "/Users/jojo/git/beets/test/helper.py", line 451, in run_command
    beets.ui._raw_main(_convert_args(list(args)), lib)
  File "/Users/jojo/git/beets/beets/ui/__init__.py", line 1291, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/Users/jojo/git/beets/beetsplug/convert.py", line 491, in convert_func
    m3ufile.write()
  File "/Users/jojo/git/beets/beets/util/__init__.py", line 184, in write
    with open(self.path, "w") as playlist_file:
FileNotFoundError: [Errno 2] No such file or directory: b'/var/folders/3t/l575q0f96fx91pp61zb158z00000gn/T/tmpettdj4j2/convert_dest/playlist.m3u8'

----------------------------------------------------------------------
Ran 1 test in 0.068s

FAILED (errors=1)
(beets) ➜  beets git:(convert_playlist)
(beets) ➜  beets git:(convert_playlist) ls -l /var/folders/3t/l575q0f96fx91pp61zb158z00000gn/T/tmpettdj4j2/
total 72
drwxr-xr-x  3 jojo  staff     96 Aug 10 07:24 libdir
-rw-r--r--  1 jojo  staff  36864 Aug 10 07:24 library.db
(beets) ➜  beets git:(convert_playlist)

I think I misunderstand something pretty basic here. Maybe you have an idea to further debug this. Thanks so much in advance!

[sampsyo]

Hmm… my best guess here is that the code currently tries to write the playlist file (i.e., does m3ufile.write()) before the actual conversion happens (self._parallel_convert()). That latter call has the responsibility to create the destination folder—in the test harness, that convert_dest directory. So maybe this would just work if the playlist file were created after that?

[JOJ0]

Hi, thanks so much @sampsyo, it's working now, some progress is to note and some new questions arise:

• I'm not sure how I would ensure the compatibility with playlists created on Windows for Windows. Some pointers to helper functions would be great.
• Would it be ok for this PR to go through if the M3UFile class "only" provides a good basis for further implementation of features required by other m3u playlist oriented plugins?
  • What would be required to achieve that? Currently I'm testing, if I haven't forgotten anything, all the features the M3UFile class is capable of, even if not all is required by the convert+playlist feature (e.g load())
  • Reason is: I'm a little afraid that trying to port all of the existing m3u file things in playlist, smartplaylist and play plugins is a lot of errorprone and delicate work. I'm not sure if I'm willing to do that and as said want to provide a good and rock solid basis for further advancing the M3UFile class. What do you think?

PS: Also added some checkboxes to the inital post for some things I consider essential.

[sampsyo]

Well, awesome! I'm glad this seems to be working.

It's a good question about Windows. I'm not a Windows expert either… I think the best route here would probably be to just assume that everything's fine on that platform (as long as we're correctly using syspath as we do elsewhere in beets) and then let the bug reports come in. 😃 Having concrete evidence of how things go wrong can really help point in the direction of the right fix, in my experience.

And sure! Starting with a special-purpose M3U-writing utility, and expanding its functionality on demand, seems like a perfectly good plan to me.

[arsaboo]

@JOJ0 sorry....was out for a conference and could not respond. I am not a maintainer....just an active user of beets and trying to add features that I care about. In any case, looks like the issue was resolved. I hope to see this feature in beets soon. I would like to use parts of it to covert playlists.

[JOJ0]

It's currently untested when this feature is used together with
the --keep-new option. The dest option seems to be mandatory, so supposedly there shouldn't be a problem with writing the playlist file to this path. I'm a little afraid to test this right now, anyone?

[JOJ0]

Interesting, now I'm getting exactly the error I was kind of hoping for. Something is different on Windows but atm I don't know how to proceed: Testfile was created on macOS Catalina, which I guess uses unicode. Windows can't read the path correctly from the file, even though I use syspath() for reading the entry.
https://github.com/beetbox/beets/runs/8009869557?check_suite_focus=true#step:7:115

       self.assertEqual(m3ufile.media_list[0],
                         '/This/is/�/path/to_a_file.mp3\n')
E       AssertionError: '/This/is/å/path/to_a_file.mp3\n' != '/This/is/�/path/to_a_file.mp3\n'
E       - /This/is/å/path/to_a_file.mp3
E       ?          ^^

[JOJ0]

Hi @sampsyo , I think this PR finally is ready for the final review. I tried to make sure that unicode files reading and writing is working on Windows by adding some Windows specific unittests. I'm still not 100% sure if I understood syspath() entirely (I don't understand why adding \\?\ makes sense and what it's purpose is). I disabled the prefix when adding file paths to the playlist. I'm not sure if syspath() would be required at all when reading in media file paths from an m3u playlist file but I did it anyway (https://github.com/beetbox/beets/pull/4399/files#diff-f20a2ab41ac3a3c83575cac2b871653999b0e4d28c27336a1f3897a004f5d6daR49)
Hopefuilly if someone experiences issues with that approach they will report it and I'll be able to fix it. Please ping me if an issue like that would come in. Thanks!

Please specifically review the Windows tests (and the corresponding fixture files) and tell me whether you think they make sense like that:
https://github.com/beetbox/beets/pull/4399/files#diff-277edfc80947a6330096d584f8c3a0e418445dbc69976834a2ea474ac856c681R65-R89

https://github.com/beetbox/beets/pull/4399/files#diff-277edfc80947a6330096d584f8c3a0e418445dbc69976834a2ea474ac856c681R108-R118

[JOJ0]

Okay, okay, I did some homework and finally read the msdn article that's linked within the syspath() function (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247.aspx) and some stackoverflow answers. I think I get it now. It enables Windows to handle paths longer than the maximum of 260 characters. Using syspath() always adds the prefix of \\?\ to paths, because it can't hurt, even when paths are shorter. Still I'm unsure if it's correct to apply it when loading an m3u playlist's entry (https://github.com/beetbox/beets/pull/4399/files#diff-f20a2ab41ac3a3c83575cac2b871653999b0e4d28c27336a1f3897a004f5d6daR49) and when adding one (https://github.com/beetbox/beets/pull/4399/files#diff-ac0584bc04d691dc3bb362557c68b83e7e71f491b7e4e0b42013e3a73cf30dd3R488).

Isn't adding this prefix the responsibility of an application that wants to read an m3u file's entry? Thus I should not use syspath() (with prefix=True) when adding to the list? (https://github.com/beetbox/beets/pull/4399/files#diff-ac0584bc04d691dc3bb362557c68b83e7e71f491b7e4e0b42013e3a73cf30dd3R488)

[JOJ0]

I don't want to leave all the clutter of the last 2 messages standing as the final messages here. All this is probably esoteric at the moment and as always there might be room for improvement. A final review and merge could be a sensible next step. Thanks a lot in advance.

[JOJ0]

I think this is finally ready to go. There is error handling, unittests for Posix and Windows, util.syspath and util.bytestring_path wrappers are used and I use it often personally for exporting from my collection. There have been reviews by experienced beeters and I tried to incorporate there suggestions. The one thing that is lacking is testing thoroughly on Windows (besides unittests). We'll see if it holds and I'll be here for fixing.

The util.m3u.M3UFile() class that's included in this PR is ready to be used in other parts of beets. Its minimal functionality is intended but I think by adding just a few features a next step could be to use it in the smartplaylist plugin, which is the one place in beets where I stole some file handling practices from ;-)

[JOJ0]

Whoops, reminder to myself: A markdown hyperlink is not a rst hyperlink. Quickfix this in master ;-)