Playlist improvements #777
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulijar
force-pushed
the
feature/playlist_improvements
branch
2 times, most recently
from
2a690a0
to
faf9eef
Compare
Having a popup menu here instead of the floating buttons opens up the possibility to add more playlist actions. Also, the 'rename' and 'delete' actions are not so commonly used, that one extra click would be any kind of a problem.
The pane shows the same information which is available via the Subsonic API for playlsits: number of tracks, total length, creation date, and comments.
It turned out that the angularjs date filter does not respect the browser locale settings. Had to fall back to using the standard javascript function Date::toLocaleString().
The UI now allows entering at most 256 characters in the playlist name fields. The limitation was anyway enforced on the DB level.
Previously, all play times were always formatted like "m:ss". This wasn't very user friendly for playlist total time, where the time may be several hours or more. Now the formatting includes also the hours part if the formatted time is more than one hour. For time periods shorter than one hour, the formatting is like before. Also, removed the integer conversion with the ceiling function in the back-end class DetailsHelper. This is basically part of the formatting, which is the responsibility of the front-end.
This argument in the REST API has not worked for ages because of changes in the internal PHP APIs. Obviously, this has not been used by anyone, but now it is fixed anyway. The other alternative would have been to remove this option.
It made little sense to show the tooltip with the playlist track count, when the user was hovering over the "more actions" button or any of the actions in the popup menu.
When exporting a playlist to file, if the selected target folder already contains a .m3u file with a name matching the playlist name, the user is now offered two options: 1. Overwrite the existing file 2. Save the file with another name. A new unique name is automatically generated by appending a number in parentheses. Also, fixed the issue that exporting to a file was possible only for the admin user (this is the default if the REST API endpoint has not been marked as "NoAdminRequired").
The file references in the file must be given using relative paths. If some of the listed files cannot be found from the user's music library, then those files are skipped. The file must have UTF8 encoding (if not plain ASCII). Probably more correct logic would be to assume Latin-1 if no other encoding is defined in the file.
It would be nice to include also the artist name on the #EXTINF fields. However, that would currently require digging the artist entities separately from the ArtistBusinessLayer. Maybe it would be better to refactor the backend so that artist names would be glued to every Track object by the TrackMapper.
We always export the playlists using UTF8 encoding, so .m3u8 is more appropriate suffix than .m3u.
There was already quite much logic related to playlist file import and export in the file PlaylistApiController, and more will be added in the next steps. Hence, this logic was extracted to a dedicated class PlaylistFileService.
When the "import from file" is used while the target playlist is being viewed, the view is now updated automatically to show the imported contents. Previously, the contents were updated only upon navigating to another view and back.
The maximum length for the playlist name is 256 characters, but the file names cannot be quite that long on ownCloud/Nextcloud. Hence, the name is of the exported playlist is truncated to 240 characters at maximum before it is used as name for the created file.
Imported .m3u files are now assumed to have iso-8859-1 character encoding and .m3u8 files are assumed to have utf-8 encoding. Both of these may be overridden by the file by supplying the extended format tag #EXTENC: followed by the encoding. The latter has been tested with utf-8, iso-8859-1, and iso-8859-2, but it should work with any encoding supported by PHP.
Both "export to file" and "import from file" now have the file symbol on the right and arrow on the left. It's only the direction of the arrow that is different between these two. Previously, the arrow pointed from left to right on both icons, and the file symbol was in the different ends of the arrow in the two icons. Having the arrow point to reverse direction makes it more clear that these two operations are the reverse operations of each other.
The REST API route `/api/playlists/file/{fileId}` can be used to parse
a .m3u or .m3u8 file, without doing anything else. Unlike on import
from a playlist file, the contained files do not have to reside within
the music library of the user. It is still checked that the files can
be found from the user's cloud storage.
This shall come into use when we add the support to open playlist files
directly from the Files app.
The playlists can be opened for playing but the UI doesn't yet indicate in any way that it is playing the tracks in playlist order instead of the folder order.
In the "playlist mode", the embedded player shows the playlist name, the ordinal of the current track, and the total number of tracks on the playlist. The layout is unaltered when a normal audio file has been opened instead of a playlist file.
For now, the sidebar tab shows just a static list of the contained tracks. All the fancy stuff is left for future development, like: - not reparsing the file, if the same playlist is already being played - highlighting the playing track - controlling playback by clicking a track on the list
Playlist file reading now happens via the module OCA.Music.PlaylistFileService both when playing the list and when viewing its contents in the sidebar. The module caches the latest loaded playlist contents. Hence, if the user starts playing a list, and then clicks the "Show list" button, the list does not need to be reloaded but can be shown immediately. Similarly, if the user first views the contents of a playlist and then decides to play it, the playing can start immediately.
It is now possible to jump to any track of a playlist by clicking it in the Playlist tab of the details sidebar. It is even possible to start playing a list which is not already playing. To make this work, the main.js of the embedded player had to be refactored so that the context reference is not used. All the needed data could be obtained from the global object OCA.Files.App.fileList, instead.
There was a bug which caused an empty/corrupted sidebar to be shown. This happened when the "Show playlist" button was pressed for such playlist which was not loaded by the Files app. This could be because of lazy loading or because the playing list was not in the currently viewed folder. The following changes were made to fix these two cases: a) If the currently viewed folder is not the parent folder of the playlist, the "Show playlist" does nothing but shows an error note. b) The target playlist file is scrolled to the viewport before opening the details pane, thus ensuring that it is already loaded by Files.
The result of the API endpoint `/api/playlists/file/{fileId}` was
changed slightly to make it more similar with the model of the Files
app. This allows us to create the file download URL the same way for the
playlist tracks as we are using on the tracks played individually in
the Files app.
The track names shown in the playlist tab view are now obtained from the file names the same way as elsewhere: the file extension is dropped, as well as any prefix which looks like a track number. Also, the track's number on the list is now shown with a gray font. This is implemented using the experimental ::marker CSS pseudo element. Currently, this works on FIrefox. On Chrome, it works only if the experimental web features are enabled from the chrome://flags.
The "import from file" and "export to file" icons were modified by adding a gray border to the arrow symbols. With this change, the same icon works sufficiently well both on the light and the dark theme.
The "more" icon on the playlist items on the navigation pane is now hidden by default. The icon becomes visible when the playlist item is hovered or the playlist in question is active. On the other hand, the icon now has a bit more contrast (higher opacity) to make it more visible.
If the track has no known artist set, then the artist text is omitted. That is, the "Unknown artist" text is not included in the exported data.
The #EXTINF fields of the m3u(8) files are now parsed along with the actual playlist entries. If the field is present and contains a track caption, then this is used as the title of the playlist item on the playlist tab view. Otherwise, the title is generated from the file name as before.
The PLS formatted playlist files can now be played in the Files music player, and imported in the full Music app. The differences of the PLS and M3U file types are completely transparent for the front-end, all the magic happens on the back-end.
paulijar
force-pushed
the
feature/playlist_improvements
branch
from
c465ed3
to
2c9c6db
Compare
In the Files music player, there is now a popup menu in the place where there used to be the "Show playlist" button. In this menu, there are two options: - Show playlist - Import to Music The second option creates a new playlist with the name of the playlist file, and imports the file's content to this list.
Showing the playlist contents in the sidebar is not possible when viewing folder other than the parent folder of the playlist. This is an architectural limitation in the sidebar of the Files app. This is now indicated by disabling the related action in the popup menu. Also, the action "Import to Music" makes no sense if the tracks of the playlist do not belong to the user music library. In such case, this menu option is now disabled. In case the menu item is disabled, a tooltip tells the reason for disabling. Furthermore, in case only some of the playlist tracks can be imported, also this is indicated by a tooltip.
Internet Explorer does not have the function Object.assign; use the jQuery alternative to work around this.
The player had got broken on publicly shared folders with the commit 552dfc0 "Files music player: Make the Playlist details tab interactive". The problem was that the OCA.Files.App is not available while viewing a public share. Hence, we need to obtain the fileList reference from the passed in context after all. It is now obtained from OCA.Files.App only in the case when the playback is started from the sidebar playlist view. The sidebar is available only within the Files app, so this is safe. Playing playlist files within the publicly shared folder still doesn't work. This is because the playlist parsing happens using the API endpoint `/music/api/playlists/{fileId}/parse` which is not public.
Do not inject the current user and folder to the constructor of PlaylistFileService because these are not available when using a public (link shared) share. Instead, inject the user and folder to each function which needs them.
For playing to work, also the tracks on the playlist must be found from the same share. For this reason, it would be pointless to support playing an individually shared playlist file.
|
The inspection completed: 9 new issues, 17 updated code elements |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR holds a multitude of sub features, aimed to enhance the playlist use. Most of them are related to supporting playlist files in M3U, M3U8, and PLS formats. Among other things, this PR resolves the issues #652 and #645.
Screen shots
Playlist context menu and the playlist details pane
Playing a playlist file in the Files app, and showing the list contents on the sidebar. Also the new popup menu for the playlist is visible.
