GNOME Bugzilla – Bug 415366
Cuesheet support
Last modified: 2020-03-17 08:31:56 UTC
It would be great if banshee could open cuesheet files. Cuesheet information here: http://wiki.hydrogenaudio.org/index.php?title=Cuesheet The most needed feature was opening a cuesheet which references a single file, as that is the ideal format for 1:1 cd backups. Thanks
Please specify the latest version of banshee you have been able to test this against or let us know that this is no longer a problem. Thank you for helping us keep track of your bug.
Maybe a good plugin idea?
It would be awesome to have .cue support in the new banshee. Many lossless audio fans out there, and growing. Typically the CD collections are ripped to a cuesheet (.cue) plus a single .ape (monkey's audio) or FLAC or wavlan (wv) file. There is no good support for cue sheets in Linux music players yet. The usual problems is: when you load/scan the disk structure, both the single file .ape (.flac, .wv) and the .cue songs are added to the playlist, effectively adding the album twice; this is a known audacious problem. When a .cue references an .ape/.flac/.wv audio file, the audio file itself should not be added to the playlist. It would make a real difference if banshee could handle cuesheets well. This is with banshee-1 beta (0.99)
Also: this bug should be changed to banshee version 1 (not the legacy branch)
CCing myself because this feature request has been brought quite often to my attention lately.
It would be great to see this feature beeing added.
A reference player with really good cuesheet support is: foobar2000 (http://www.foobar2000.org/) which installs and runs fine with wine (ubuntu 8.10 / amd64) When building its media library, it recognizes the cuesheet files, indexes the tracks described inside the cuesheet, and does not add the raw .flac (or .ape or .wav) that the cuesheet makes reference to. While playing, it can jump from track to track, display the name of the song, etc., with per-album .flac files. I can't wait to see this supported in banshee. All my CD collection is now in my media server / PC in the form of per-album .flac files + cuesheet.
Count me in for those willing to have support for it.
I'm more interested in who is willing to *add* support for it.
*** Bug 537901 has been marked as a duplicate of this bug. ***
This is the #1 bug, for me. Until Banshee supports Embedded Cues, I can't use Banshee. :( I'm thinking of switching back to Linux from Windows 7 and it's very sad that I wont be able to use my favorite music player (Banshee) because of the way I have my music saved..
Is anyone working on this feature? I have also lots of flac/cue albums and Banshee looks otherwise quite decent. Can this be implemented as an extension?
See https://bugzilla.gnome.org/show_bug.cgi?id=691373
*** Bug 691373 has been marked as a duplicate of this bug. ***
Created attachment 233304 [details] [review] Patch from Hans for the necessary changes in Banshee itself This a patch by Hans from the duplicate bug. It has some issues, and I've started working on it.
Created attachment 233305 [details] [review] Patch from Hans for the new extension for CueSheet support The second patch from the duplicate. I'm also looking at it, and will probably merge it into the Community Extensions when I'm done.
As I mentionned, I've started looking into the patches. In the meantime, as I don't use cue sheets myself, if anyone could point me to some free samples of music files with cue sheets, that would really help my testing.
Hi Bertrand, I've been developing further. I haven't changed anything in the Banshee code anymore, but I have done a lot of work in the CueSheets code. I'm adding a tar.gz with the latest sources, as I don't have access to the git master repository and this is 'new' code in the extension. The CueSheet extension builds up a database of cuesheets as follows from a base directory: <Cuesheet base directory>/<genre 1>/<directories with .cue files> /<genre 2>/<directories with .cue files> /... If you need a sample directory, you can lend some of my music. I'm also willing to support al of the code and you could review it, but for that I would need access to the git repository as a developer. Thanks for supporting this. Hans
Created attachment 233324 [details] CueSheets extension - 0.0.3
Created attachment 233347 [details] CueSheets extension 0.0.4
I added preferences code and removed the superfluous buttons for configuration and about dialog.
Hans, I've added you to the team on gitorious, you should now be able to push to the repository: https://gitorious.org/banshee-community-extensions/banshee-community-extensions If you prefer, you can also clone the repository, push your changes to your clone, and request a merge when you're ready. I'll be happy to review your changes.
Created attachment 233350 [details] CueSheets extension 0.0.5
0.0.5 Made the extension more robust. Playing while nothing has been selected or while nothing has been activated, but one selected is working. Next work will be using the repository.
(In reply to comment #22) > Hans, I've added you to the team on gitorious, you should now be able to push > to the repository: > https://gitorious.org/banshee-community-extensions/banshee-community-extensions > > If you prefer, you can also clone the repository, push your changes to your > clone, and request a merge when you're ready. I'll be happy to review your > changes. Ok. Probably somewhere next week I'll work on that. Thanks. I need to get accustomed to git. Haven't got much experience with git. Will you be adding the needed changes to the banshee core?
I pushed my CueSheets changes back to the banshee-community-extensions git repository. Please be welcome to review the changes. I'd like to add some more features in the future like cuesheet editing. Also filling the 'database' is a process that should happen in the background. Especially if the number of .cue files is big. An other interesting option would be to be able to play tracks randomly in the cuesheet extension of create playlists with tracks all over the cuesheets. For now this is not possible. Thanks in advance for reviewing the code.
Beside the code changes in the banshee repository, I also would embrace the following code change in AlbumListView.cs. Instead of: private bool DisabledAlbumGrid ... I'd like to have public bool DisabledAlbumGrid ... This makes it possible to reuse the AlbumListView in extensions and also its functionality to change between grid and list layout.
Created attachment 233403 [details] [review] Superfluous import of bp_play_at; private member of AlbumListView made public
Created attachment 233460 [details] [review] Patch to be able to toggle grid/list layout for the album view This private member did set a global Setting for Banshee. I needed to change the code in order to be able to set the grid/list layout without making it a global setting. See attached patch.
Created attachment 233469 [details] [review] Album listview - Now grid/list is nicely synchronized. There still a problem with the cell rendering. I don't know where to search.
I've just committed two of the changes in Banshee to git master: Accurate seek http://git.gnome.org/browse/banshee/commit/?id=fbbfb6487 AlbumListView: Allow subclasses to switch between album grid and list view http://git.gnome.org/browse/banshee/commit/?id=8bd2b8ee2 As you can see, the API is different from the proposed patches, so the CueSheets extension will have to be adapted for that. I'll attach the two remaining changes as separate patches, as I have some questions about those changes.
Created attachment 233957 [details] [review] PlayerEngine: Allow the current track to be overridden I guess this SetCurrentTrack method is to make sure the current track matches the current CueSheetEntry. But overriding the current track like that seems dangerous to me. PlayerEngine probably assumes it has control over the current_track field. I'm also worried that the CueSheets extension seems to call it every 2 seconds. Is that really necessary ?
Created attachment 233958 [details] [review] PlaybackControllerService: Add SeekMode property I'm note sure what's the purpose of this SeekMode property. Depending on what you want to achieve, an additional parameter to the RestartOrPrevious method might be better. As a side note, I'm not sure why that RestartOrPrevious method has a bool parameter which is not used. Some investigation is needed...
(In reply to comment #33) > Created an attachment (id=233958) [details] [review] > PlaybackControllerService: Add SeekMode property > > I'm note sure what's the purpose of this SeekMode property. > Depending on what you want to achieve, an additional parameter to the > RestartOrPrevious method might be better. > > As a side note, I'm not sure why that RestartOrPrevious method has a bool > parameter which is not used. Some investigation is needed... With The SeekMode property, I'm convincing an instance of the PlaybackControllerService that it must really call the 'Previous' method, instead of thinking that it has to replay a file. That's because CueSheets are mostly used to split one large mp3 or flac or ape file in chunks. I think a call parameter might indeed be better.Maybe an overload of the method?
(In reply to comment #34) > (In reply to comment #33) > > Created an attachment (id=233958) [details] [review] [details] [review] > > PlaybackControllerService: Add SeekMode property > > > > I'm note sure what's the purpose of this SeekMode property. > > Depending on what you want to achieve, an additional parameter to the > > RestartOrPrevious method might be better. > > > > As a side note, I'm not sure why that RestartOrPrevious method has a bool > > parameter which is not used. Some investigation is needed... > > With The SeekMode property, I'm convincing an instance of the > PlaybackControllerService that it must really call the 'Previous' method, > instead of thinking that it has to replay a file. That's because CueSheets are > mostly used to split one large mp3 or flac or ape file in chunks. > > I think a call parameter might indeed be better.Maybe an overload of the > method? However, the problem may be that I've got no control over the next/previous button call sequence and that's why I chose a Property.
(In reply to comment #32) > Created an attachment (id=233957) [details] [review] > PlayerEngine: Allow the current track to be overridden > > I guess this SetCurrentTrack method is to make sure the current track matches > the current CueSheetEntry. > But overriding the current track like that seems dangerous to me. PlayerEngine > probably assumes it has control over the current_track field. > > I'm also worried that the CueSheets extension seems to call it every 2 seconds. > Is that really necessary ? I want to have more control over the current track, because the file doesn't change. So I set the current track myself. It seems to work well, but may indeed not be the intended behaviour. I'm not calling the method every two seconds. I only check every two or three seconds if I should change the current track display. Only when things actually change, I change the track.
(In reply to comment #36) > (In reply to comment #32) > > Created an attachment (id=233957) [details] [review] [details] [review] > > PlayerEngine: Allow the current track to be overridden > > > > I guess this SetCurrentTrack method is to make sure the current track matches > > the current CueSheetEntry. > > But overriding the current track like that seems dangerous to me. PlayerEngine > > probably assumes it has control over the current_track field. > > > > I'm also worried that the CueSheets extension seems to call it every 2 seconds. > > Is that really necessary ? > > I want to have more control over the current track, because the file doesn't > change. So I set the current track myself. It seems to work well, but may > indeed not be the intended behaviour. > > I'm not calling the method every two seconds. I only check every two or three > seconds if I should change the current track display. Only when things actually > change, I change the track. So really, I only want to make sure the player engine displays the right track of the cuesheet. Banshee only changes this display if the file changes, but with cuesheets there's no 1:1 relation between file and track, it's a 1:n relation.
(In reply to comment #36) > (In reply to comment #32) > > Created an attachment (id=233957) [details] [review] [details] [review] > > PlayerEngine: Allow the current track to be overridden > > > > I guess this SetCurrentTrack method is to make sure the current track matches > > the current CueSheetEntry. > > But overriding the current track like that seems dangerous to me. PlayerEngine > > probably assumes it has control over the current_track field. > > > > I'm also worried that the CueSheets extension seems to call it every 2 seconds. > > Is that really necessary ? > > I want to have more control over the current track, because the file doesn't > change. So I set the current track myself. It seems to work well, but may > indeed not be the intended behaviour. > > I'm not calling the method every two seconds. I only check every two or three > seconds if I should change the current track display. Only when things actually > change, I change the track. Replying to myself. I think the right solution for this problem is following: IBasicPlaybackController asks me to implement the 'Previous' method. But when I implement this method in my Source, it is never called by the PlayerEngine. The 'Next' method is. It is never called, because the PlayerEngine thinks it should restart the track by itself instead of letting the derived interface decide what to do. However, the Previous method can be called with a 'restart' flag. So, why is it never called? Maybe because of legacy code that behaves differently? A solution could be to extend the interface with a property, i.e. MustCallPreviousAlways, or change the behaviour of calling the 'Previous' method, which I would think is the most pure way of doing it. Both changes may require others to look at their code. However, maybe the dependencies aren't so big.
I created a cuesheets solution using most of the present functionality. This prevents complete integration in the music library, because the present music library is written for the playback of URIs. If the core playback functionality could be rewritten to be able to play from begin offset to end offset, it would be quite easy to integrate cuesheets in the main music library. Would this be a good idea?
Created attachment 234188 [details] [review] Integrating cuesheet support into the main banshee music library I've been working further on the idea of integrating cuesheet support into the standard music library of banshee. It's quite tricky. Banshee is track oriented and doesn't expect to have multiple tracks in one file. The included patch works quite well. As tracks are now integrated into the main music library, all functionality of this library is available. Including playlists, shuffle, etc. There are some issues with VBR mp3s and getting the right position back in an MP3 file while it's playing, but that's gstreamer related and not a fault of banshee. CBR mp3s and FLAC files work well. The code isn't very beautiful, because there's a timed transaction that takes control over the playback of a 'cue-ed' file, if a track is part of a cue. However, using gst_element_seek to set a play-range, didn't work, because it gives problems with the very base idea of using cuesheets and that's that going from one track to the next doesn't give a noticeable gap.
>+ BeginOffset INTEGER, >+ EndOffset INTEGER, >+ CueAudioFile VARCHAR, >+ PartOfCue INTEGER > ) Shouldn't you add this in a new table, and then have a foreign key to the main one? (Sorry Bertrand for jumping in! Just a quick comment, I won't step on your toes :) )
That is possible. The code I posted is just a quick first finger exercise to constitute the very idea. When adding support in the real 2.7 version of banshee, this code probably has to be much more robust. One of the main problems in this code is that gstreamer (assumably by design, see https://bugzilla.gnome.org/show_bug.cgi?id=692368) doesn't reliably report the current position in a stream. So support for cuesheets (or indexed audio files) might need to be incorporated much lower in the banshee code. (EOS, etc.). I have tried this path, but I have no clue about how to proceed. Not enough knowledge of the internal workings.
Created attachment 234223 [details] [review] Small error in the previous patch
Created attachment 234402 [details] [review] Integrating indexing/cuesheet support in the banshee core I think this is a reasonable way, given gstreamer 0.10, to integrate indexing or cuesheet support into banshee. If the banshee core can work like this, the cuesheet community extension only needs to provide importing cuesheets into the banshee music library and maybe some way to edit the base cuesheets.
See also https://bugzilla.gnome.org/show_bug.cgi?id=692368. I tried to rely on gstreamers SEGMENT_DONE message and reseeking to a new segment, but this didn't work at all, because there's what I think a bug in gstreamer 0.10. I don't know the status of gstreamer 1.x
(In reply to comment #44) > Created an attachment (id=234402) [details] [review] > Integrating indexing/cuesheet support in the banshee core > > I think this is a reasonable way, given gstreamer 0.10, to integrate indexing > or cuesheet support into banshee. > > If the banshee core can work like this, the cuesheet community extension only > needs to provide importing cuesheets into the banshee music library and maybe > some way to edit the base cuesheets. There's one issue left in this patch, and that's importing music into the library. If banshee could see tracks with the same url as different (which could be done using BeginOffset and EndOffset to distinguish the parts) this would make 'CueAudioFile' redundant and also other compensating factors in the CueSheets extension, like creating symbolic links to the same file to make urls different.
(In reply to comment #45) > See also https://bugzilla.gnome.org/show_bug.cgi?id=692368. I tried to rely on > gstreamers SEGMENT_DONE message and reseeking to a new segment, but this didn't > work at all, because there's what I think a bug in gstreamer 0.10. I don't know > the status of gstreamer 1.x There's an issue in my workaround for this code. I use a g_timeout to gard the position of the audiostream for the player struct. But who's to say that the memory for this player struct isn't discarded before the next timout callback event? So banshee might crash this way. I think I need to connect to the audio stream using an extra sink on the tee :-(
Created attachment 234522 [details] [review] Segmented playing in banshee Going further with integrating indexing/cuesheet support, I have now defined playback of segments of media in Banshee. I think this is the structural approach to integrate segmented playback in banshee.
(In reply to comment #48) > Created an attachment (id=234522) [details] [review] > Segmented playing in banshee > > Going further with integrating indexing/cuesheet support, I have now defined > playback of segments of media in Banshee. I think this is the structural > approach to integrate segmented playback in banshee. So this patch must be applied along with the previous one.
Created attachment 234528 [details] [review] Duration needs to be set when using offsets.
Created attachment 234530 [details] [review] Duration needs to be set properly
Created attachment 234557 [details] [review] FileSize needs to be adjusted to the size of the segment if appropriate.
hi, Just a small feedback, if you modify the libbanshee which is the gstreamer backend. Can you modify the gst# backend to add the same behaviour: http://git.gnome.org/browse/banshee/tree/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp
I probably can. I'll have to look into the matter. But I'd first like to have feedback concerning the contents on the current patches.
Thanks for the patches ! I haven't had time to dig into them yet, so here's some feedback, after quickly scanning through them, so it's only going to be on the form, not the substance : - Please don't add revision markers like "// HOD". Version control makes it already easy to find who wrote what :) - Please follow the code formatting guidelines in the HACKING file. - Some patches appear to be changes on top of previous patches. If you're improving the initial patch, please post a new complete patch and mark the previous patch as obsolete. This makes it easier to see what needs to be reviewed. A few words on my point of view here: I think cuesheet support is a valid use case, but not a major one, like "playing a regular mp3". So basically, I'd like to be convinced that it's not going to break anything or cause regressions. It might be easier to achieve this with several small patches with explanations in the commit messages, than with one big patch. I'm going to be away for most of the next 2 weeks, so anyone who want to dig into this is most welcome !
Dear Bertrand, Thanks for your comment on the subject. I know cuesheet support is a big deal for (a minority of) people. Often because they have a large music collection in that format, which they created because of gap issues with mp3 playback. I'm not sure what you want me to do now. I can recreate the patches I've made sofar with comments on what I'm doing, if that's what you want. I know these initial patches will give some more work with e.g. transporting tracks to playback devices like iPods (because then you want to split the large album files). However, the patch that I've been working on now, is basically only a change in the banshee playback mechanism to playback tracks that are a 'segment of a piece of music', or the old 'a to b' playback on your cd player. So it's very generic support for playback of segments of files. I don't like the idea of creating a separate git repository of banshee away from the main branch. If I'm going to contribute my work for banshee, I want to contribute it to the main distribution. As you may have noticed, I maybe not following the HACKING guidelines to the letter, but I'm not a beginner programmer, even though this has been my first contribution in C#. So I've got a couple of questions: - First of all: Do you want this contribution? - If so, how do you want my patches so that they are acceptable? - Are you or the banshee testers willing to invest in regression testing, because my patches will inevitably break something? Thanks in advance for your answers. Thanks in advance for your answer.
Ok, I'm following instructions now for a large patch (see https://live.gnome.org/Git/Developers#Publishing_your_tree) (got some mail). I will follow the remark: "It might be easier to achieve this with several small patches with explanations in the commit messages, than with one big patch." And make small patches to https://github.com/hoesterholt/banshee-segments.git Anyone who wants to help me on this work is welcome. I'll post the patches explanations on this bug.
I have added the patches to the github tree. https://github.com/hoesterholt/banshee-segments/commit/18cd3eb2646377dc71b6a0ef1cb64d095ea56c84 I also created some documentation on the patch. This is my contribution to banshee for now.
Created attachment 234976 [details] Checklist for integration support for playback of segments
(In reply to comment #59) > Created an attachment (id=234976) [details] > Checklist for integration support for playback of segments Wow, that's awesome, good work! (In reply to comment #58) > I have added the patches to the github tree. > > https://github.com/hoesterholt/banshee-segments/commit/18cd3eb2646377dc71b6a0ef1cb64d095ea56c84 That's really great work. But the patch is huge, do you think it could be broken down in different patches? For example, maybe the ideal thing to do is have the minimal set of patches required for CueSheets extension to run, be committed for the 2.7 release, and maybe after 2.7.0 is released, we can integrate the rest that provide more enhancements to the extension? Otherwise, the review is going to be very long and hard. Also, I've noticed you have forgotten to correct some things in your patch that have already been pointed out: like the "HUD" comments, the modification of the SQL table (let's rather have a different table with a foreign key, created by the CueSheets extension), and the style guidelines [1] (for example, you forgot to use a space before the character of an opening bracket). By doing another quick look, I've noticed you also sometimes add commented code (if it's commented, it should be removed really, unless you state with a reason why it's there). Your copyright notices are incomplete: +// Author: +// hans <${AuthorEmail}> +// +// Copyright 2013 hans Can you follow the format of the other files? There are also some things unrelated to this in the patch, like the modification of the file translators.xml and some changes in .csproj files (like removing of warning levels). I know this is not your fault, but banshee's build and MonoDevelop itself, but it's much better if you could remove them (even manually) from the patch. Oh, and please, post the patch here, not in github. You can still work in github if it's more comfortable to you, but later you can go on to the command line and use "git format-patch" to extract the patch file to post here, like it is adviced in our development guidelines [2] (section "Producing a patch"). Understand that we cannot treat each contributor differently, otherwise it would be a mess! So if in this 2 weeks that Bertrand is away, you address these issues I just mentioned, and post a new patch, it would be much appreciated, and Bertrand will be much more comfortable reviewing it. Thanks so much for making Banshee better! [1] http://git.gnome.org/browse/banshee/tree/HACKING [2] http://banshee.fm/contribute/write-code/
> > That's really great work. But the patch is huge, do you think it could be > broken down in different patches? For example, maybe the ideal thing to do is > have the minimal set of patches required for CueSheets extension to run, be > committed for the 2.7 release, and maybe after 2.7.0 is released, we can > integrate the rest that provide more enhancements to the extension? > I'm sorry, it is a big job to change the default way banshee thinks about tracks. It's I think quite a fundamental change to introduce tracks that are segments into media files as the granularity level instead of tracks that are equal to files. So there are changes needed at several locations in banshee and then I haven't looked at transportation of segments of tracks to personal media players (personal audio devices). But otherwise of course I can clean up the code for you. And thank you for your positive reply.
(In reply to comment #60) > That's really great work. But the patch is huge, do you think it could be > broken down in different patches? By the way, with this sentence above I'm not asking for the opposite that Bertrand asked for: Bertrand was just concerned that there were many patches, but most of them would be additions to previous patches (that is, patches depending on previous patches). Now we're progressing because we have a patch that doesn't depend on anything. The ideal next thing is to have different patches, each one with a clear different goal, and inter-dependant from each other. Does that make sense? Sorry if this seems like a very complicated process but it's the only one we have to make reviews easier for us, and reviews are really needed if we want to maintain the quality level of the code we already have! Looking forward for the next steps.
I propose to do this when Betrand is back. Maybe you can help him in the process. I propose to follow the checklist then, which I added some minutes ago. So I can break up the process as follows: - First add the necessary elements to the data layer (TrackInfo related) - Next focus on import of segmented tracks (this makes testing possible) - Next add the changes needed to playback control and gui. - Lastly, the most nasty stuff, build up the playback engine related stuff: - beginning with libbanshee, gstreamer backend and mediaengine.playerengine - next the gstreamer-sharp backend However, all elements of this patch are needed in order let banshee work correctly. - Audio device synchronization can be done later.
Banshee is not under active development anymore and had its last code changes more than three years ago. Its codebase has been archived. Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect reality. Please feel free to reopen this ticket (or rather transfer the project to GNOME Gitlab, as GNOME Bugzilla is being shut down) if anyone takes the responsibility for active development again. See https://gitlab.gnome.org/Infrastructure/Infrastructure/issues/264 for more info.