GNOME Bugzilla – Bug 623155
Moving audiobooks to the Audiobook Library doesn't result in files being moved to the Audiobook folder when copy on update is set.
Last modified: 2020-03-17 08:56:32 UTC
Dragging audiobook entries from the Music library to the Audiobook Library doesn't result in files being moved to the Audiobook folder when copy on update is set as would be expected.
The preference "Copy files to media folders when importing" just says "Copy", not "Move". I don't think we should move stuff without the user knowing. Maybe you are referring to the option "Update file and folder names", which suggests a "Rename" of the folder, which is basically a Move (not a Copy). Maybe we should re-phrase that preference. BTW Samuel just pointed me out on IRC that this preference has a mixture of names: in the UI we call it "Update file and folder names", but in the code it's called MoveOnInfoSave (LibrarySchema) and RenameEnabled (Service).
Created attachment 229828 [details] [review] Patch to fix movement between two libraries What is done: 1. Enabled "Update file and folder name" on library. 2. Enabled moving files between two libraries not sharing the same folder 3. Enabled moving files between two libraries that are sharing the same folders (files getting moved to the scheme of the target library). 4. Fixed moving of files outside the music library based on tag changes. What is not done: 1. Automatic move from files to audiobook library if they get marked as audiobook. (don't know if core developers would like this) 2. Mark a file as audiobook when it gets moved to the audiobook library. (e. g. set its main genre to "Audiobook" on move. Again I don't know if core developers would like this.) I only checked this changes with music and audiobook library... Happy testing..
To test the fix you have to enable the following two options in Banshee: 1. General -> Sync metadata between library and files 2. Source Specific -> Update file and folder names (this one on source and target library of the move, e. g. "Music" AND "Audiobook")
Thanks for your patch Ben. Some questions in line: (In reply to comment #2) > Created an attachment (id=229828) [details] [review] > Patch to fix movement between two libraries > > What is done: > > 1. Enabled "Update file and folder name" on library. Mmm, is this part really related to this bug? Take in account that this option refers to file name and folder name *within* the library. For instance, I have this song: /home/knocte/Music/Mike Jackson/Thriller.mp3 If I update the artist to be "Michael Jackson", the file should get moved to: /home/knocte/Music/Mike Jackson/Thriller.mp3 So maybe what we're learning here first is that we should rename the setting "Update file and folder names" to "Update file and folder names when metadata changes". So, this setting is not about Source changes, is about metadata. > 2. Enabled moving files between two libraries not sharing the same folder I understand this is the main issue this patch should fix, not the other ones, right? > 3. Enabled moving files between two libraries that are sharing the same folders > (files getting moved to the scheme of the target library). Sorry, I don't understand this one, can you put an example? > 4. Fixed moving of files outside the music library based on tag changes. Put an example about this one too please.
(Typo, the second path I wrote should be /home/knocte/Music/Michael Jackson/Thriller.mp3 obviously)
Hi Andrés, you're right with this. "Update file and folder names" is related to files that are already in the library (which isn't directly part of this bug). The problem is that I have to ensure that the user really wants to move files between folders when he drags a file from one library to another. I thought of two possible solutions: 1. Ask the user if he wants two move during the drap operation. 2. Check settings if user has enabled some options to move files. As the first solution means some big changes in low level code (you have to add a parameter to all options that are called during the file move to reflect the users choice), I have choosen the second one. As there is no other option in settings I enabled "Update file and folder names" for the audiobook library and checked if this option (which somehow indicates a move) is enabled on both libraries (active library and target library of the files being moved). Only if both libraries are set up to move files, the files are being moved between folders. (In reply to comment #4) > Thanks for your patch Ben. Some questions in line: > > > Mmm, is this part really related to this bug? > > Take in account that this option refers to file name and folder name *within* > the library. > > For instance, I have this song: > > /home/knocte/Music/Mike Jackson/Thriller.mp3 > > If I update the artist to be "Michael Jackson", the file should get moved to: > > /home/knocte/Music/Mike Jackson/Thriller.mp3 > > So maybe what we're learning here first is that we should rename the setting > "Update file and folder names" to "Update file and folder names when metadata > changes". > > So, this setting is not about Source changes, is about metadata. > So as said before, I totally agree with you here. > > > 2. Enabled moving files between two libraries not sharing the same folder > > I understand this is the main issue this patch should fix, not the other ones, > right? It is one part of the main issue (second part is 3.), you will see with an example for 3. > > > > 3. Enabled moving files between two libraries that are sharing the same folders > > (files getting moved to the scheme of the target library). > > Sorry, I don't understand this one, can you put an example? > You can set up a folder for each library (e. g. music or audiobook) and there is no restriction to the setting. So a user could maybe set the same folder for music library and audiobook library (which shouldn't be a problem). Sadly after looking at the code there are more fixes necessary to fix both scenaries (1. both sources sharing the same folder and 2. both sources have a separate folder). There a two methods that play together right now. 1. IteratorCore in SaveTrackMetadataJob.cs (hope the name is correct as I don't have the code at the moment), which is responsible two rename the files after metadata changes (General -> Sync metadata...). 2. AddTrack(s) in LibrarySource.cs (again I hope I remember the name correctly), which is responsible to add new files to a library (it's called on the new LibrarySource when files are being dragged). So I had to change both methods to get the expected result. IteratorCore to get the files moved correctly when both libraries share the same folder (AddTrack fails here because it only checks for the correct base path which is already ok as both share the same base path). (Side note: Files should still get moved as there is a good chance that different sources have different naming conventions set up.) AddTrack to get files moved correctly when both libraries doesn't share the same folder. In this case we only could copy the files in current code. So I added code to move files to the correct folder. > > > 4. Fixed moving of files outside the music library based on tag changes. > > Put an example about this one too please. This one isn't directly related to this bug, but it's necessary to fix it to get this fix work correctly. If you look in the current code for IteratorCore in SaveTrackMetadataJob.cs it also checks if a file has to be moved after metadata is written back to it. Sadly it's hard coded to the MusicLibrarySource, which means it first checks if the file is already in the MusicLibrarySource (otherwise it rejects the move), and then moves the file to the current naming scheme of MusicLibrarySource. So, the IteratorCore is already called for every source, but will fail to move files on all other sources except MusicLibrarySource. I think we should fix this too, as it makes sense to also move files in the audiobook source folder when metadata is changed... Hope to make things a bit clearer. Thanks for you suggestions! :D
I've been thinking that maybe the whole thing could be simplified greatly by simply having: [X] Let Banshee manage media Which would enable all the moving jive and let Banshee set a default layout such as ~/Media/Music, ~/Media/Video, ~/Media/Audiobooks, ~/Media/Podcasts. We might want have some UI to set the base directory for this so that users could say keep media on an external HD or on a NAS but letting Banshee manage everything beyond that seems like the sanest option with the fewest corner cases. I'd even lobby for this to then be the default setting, it would solve a shit ton of problems and give some dependability to the setup. However to be able to set that as the default we'd have to be capable of doing all the move on metadata change goodness on all sources. Regardless I am really happy to see someone give some love to our Audiobook source, it sorely needs it and I will happily buy you a beer kind gentleman for your efforts.
(In reply to comment #7) > I've been thinking that maybe the whole thing could be simplified greatly by > simply having: > > [X] Let Banshee manage media > > Which would enable all the moving jive and let Banshee set a default layout > such as ~/Media/Music, ~/Media/Video, ~/Media/Audiobooks, ~/Media/Podcasts. That sounds great, I would advocate that. But doesn't that go against some form of already-established Gnome convention/standard? AFAIK when you install a Linux distro this days, it already comes with $HOME/Music and $HOME/Video and the like, created for you, and with shortcuts in your $file_explorer, no? I would prefer that we follow those standards. And, if you want to propose changing them, I'll give my +1 to that, but in a separate bug...
(In reply to comment #6) > Hi Andrés, > > you're right with this. "Update file and folder names" is related to files that > are already in the library (which isn't directly part of this bug). The problem > is that I have to ensure that the user really wants to move files between > folders when he drags a file from one library to another. > > I thought of two possible solutions: > 1. Ask the user if he wants two move during the drap operation. > > 2. Check settings if user has enabled some options to move files. I also thought about those 2. Like you, I didn't like the 1st one much. > As the first solution means some big changes in low level code (you have to add > a parameter to all options that are called during the file move to reflect the > users choice), I have choosen the second one. However, for the 2nd option we have to be very careful here about what option we choose to use: a new one? if not a new one, will the behaviour be expected for veteran Banshee users as well as new ones? > As there is no other option in settings I enabled "Update file and folder > names" for the audiobook library and checked if this option (which somehow > indicates a move) is enabled on both libraries (active library and target > library of the files being moved). Only if both libraries are set up to move > files, the files are being moved between folders. Sounds fair enough, but maybe we need to rephrase the setting name then. Any suggestions? > > > > > 4. Fixed moving of files outside the music library based on tag changes. > > > > Put an example about this one too please. > > This one isn't directly related to this bug, but it's necessary to fix it to > get this fix work correctly. > > If you look in the current code for IteratorCore in SaveTrackMetadataJob.cs it > also checks if a file has to be moved after metadata is written back to it. > Sadly it's hard coded to the MusicLibrarySource, which means it first checks if > the file is already in the MusicLibrarySource (otherwise it rejects the move), > and then moves the file to the current naming scheme of MusicLibrarySource. > > So, the IteratorCore is already called for every source, but will fail to move > files on all other sources except MusicLibrarySource. > > I think we should fix this too, as it makes sense to also move files in the > audiobook source folder when metadata is changed... Agreed, but there is a reason why it's only hardcoded to the MusicLibrarySource now: because it hasn't yet been implemented for the other kind of sources. And why it hasn't been implemented? Because I don't believe we have yet infrastructure to parse special Content-related tags, such as: Episode (for Video), Book Title & Author (for Audiobooks), etc. So I think we should split things up a bit here. If the bugfix for this bug really depends on this scenarios to be fixed first, we need to split the patches for each functionality (a good acceptance criteria would be to have a varying "track editor" GUI depending on the kind of track we're editing; I think this makes me think there is already a bug open for this). Don't you think so?
(In reply to comment #8) > (In reply to comment #7) > > I've been thinking that maybe the whole thing could be simplified greatly by > > simply having: > > > > [X] Let Banshee manage media > > > > Which would enable all the moving jive and let Banshee set a default layout > > such as ~/Media/Music, ~/Media/Video, ~/Media/Audiobooks, ~/Media/Podcasts. > > That sounds great, I would advocate that. > > But doesn't that go against some form of already-established Gnome > convention/standard? AFAIK when you install a Linux distro this days, it > already comes with $HOME/Music and $HOME/Video and the like, created for you, > and with shortcuts in your $file_explorer, no? > > I would prefer that we follow those standards. And, if you want to propose > changing them, I'll give my +1 to that, but in a separate bug... There is a standard for these in Freedesktop that is true and GNOME uses them. The win we get is translated directory names for Music and Video. There are no such directories for Audiobooks or Podcasts. They also only exist on platforms where implemented (read: Linux and friends). They are not shortcuts, they are actual directories and we rename them based on the language of the session at log in time only. You will be offered to move the files if you log in using another language. It has some fragility and it tries to solve something that I don't think is a problem. The user should not care about the naming convention of his files nor the structure of the directories. We merely pick a nice human readable layout because it is easy and it allows for the user to interact with the collection directly when absolutely needed. However the primary way to interact with media should be via the media player where we have much nicer and more portable ways to display this to the user that does not have any of the downsides. The whole point of having a managed library is that you shouldn't have to care about it, hence the use of a media top level name. The standard is poor, I can try to request it be changed but I have a feeling that is a long shot. It also only applies to one platform we run on and my opinion is that it is fragile and unneeded. However I will take the request upstream with regards to having added directories for uses cases beyond music and media.
(In reply to comment #7) > I've been thinking that maybe the whole thing could be simplified greatly by > simply having: > > [X] Let Banshee manage media I also would like to make things more easy and clear in the settings (at the moment there are a couple of options that aren't always clear). > > Which would enable all the moving jive and let Banshee set a default layout > such as ~/Media/Music, ~/Media/Video, ~/Media/Audiobooks, ~/Media/Podcasts. > > We might want have some UI to set the base directory for this so that users > could say keep media on an external HD or on a NAS but letting Banshee manage > everything beyond that seems like the sanest option with the fewest corner > cases. I really like how hits handled now (you are able to setup a folder for each library that manages files). As someone suggested we could easily use some already existing standards and use those as default settings for the folders. Using this people doesn't have to care about the folders, but are able to change them if they like. > > I'd even lobby for this to then be the default setting, it would solve a shit > ton of problems and give some dependability to the setup. However to be able to > set that as the default we'd have to be capable of doing all the move on > metadata change goodness on all sources. In the long run we should implement that on all sources. But for there shouldn't be a problem with implementing it piecewise (current code also handles it only partly). Maybe it will also enable us to move MusicLibrarySource and VideoLibrarySource to extensions as all other libraries (someone already created a bug to try moving VideoLibrarySource to extensions). I would like to see that as then all Libraries could be handled the same... > > Regardless I am really happy to see someone give some love to our Audiobook > source, it sorely needs it and I will happily buy you a beer kind gentleman for > your efforts. Happy to hear that you like it :D.
(In reply to comment #9) > (In reply to comment #6) > > Hi Andrés, > > > > you're right with this. "Update file and folder names" is related to files that > > are already in the library (which isn't directly part of this bug). The problem > > is that I have to ensure that the user really wants to move files between > > folders when he drags a file from one library to another. > > > > I thought of two possible solutions: > > 1. Ask the user if he wants two move during the drap operation. > > > > 2. Check settings if user has enabled some options to move files. > > I also thought about those 2. Like you, I didn't like the 1st one much. > > > > As the first solution means some big changes in low level code (you have to add > > a parameter to all options that are called during the file move to reflect the > > users choice), I have choosen the second one. > > However, for the 2nd option we have to be very careful here about what option > we choose to use: a new one? if not a new one, will the behaviour be expected > for veteran Banshee users as well as new ones? > > > > As there is no other option in settings I enabled "Update file and folder > > names" for the audiobook library and checked if this option (which somehow > > indicates a move) is enabled on both libraries (active library and target > > library of the files being moved). Only if both libraries are set up to move > > files, the files are being moved between folders. > > Sounds fair enough, but maybe we need to rephrase the setting name then. Any > suggestions? > > > > > > > > > 4. Fixed moving of files outside the music library based on tag changes. > > > > > > Put an example about this one too please. > > > > This one isn't directly related to this bug, but it's necessary to fix it to > > get this fix work correctly. > > > > If you look in the current code for IteratorCore in SaveTrackMetadataJob.cs it > > also checks if a file has to be moved after metadata is written back to it. > > Sadly it's hard coded to the MusicLibrarySource, which means it first checks if > > the file is already in the MusicLibrarySource (otherwise it rejects the move), > > and then moves the file to the current naming scheme of MusicLibrarySource. > > > > So, the IteratorCore is already called for every source, but will fail to move > > files on all other sources except MusicLibrarySource. > > > > I think we should fix this too, as it makes sense to also move files in the > > audiobook source folder when metadata is changed... > > Agreed, but there is a reason why it's only hardcoded to the MusicLibrarySource > now: because it hasn't yet been implemented for the other kind of sources. And > why it hasn't been implemented? Because I don't believe we have yet > infrastructure to parse special Content-related tags, such as: Episode (for > Video), Book Title & Author (for Audiobooks), etc. Ok, didn't think of this. But I think that shouldn't be a problem as current code checks for options like "HasCopy" or "HasMove" which shouldn't be enabled on libraries that aren't ready to handle files correctly. Are there any other libraries (except MusicSourceLibrary and VideoSourceLibrary) that already use one of those options? > > So I think we should split things up a bit here. If the bugfix for this bug > really depends on this scenarios to be fixed first, we need to split the > patches for each functionality (a good acceptance criteria would be to have a > varying "track editor" GUI depending on the kind of track we're editing; I > think this makes me think there is already a bug open for this). Don't you > think so? It is necessary as it is otherwise not possible to move files correctly (to the scheme of the audiobook library) when the audiobook libraries folder is set to the same folder as music library. It would also break the move in a different folder that should happen after metadata changes (which isn't a problem with mp3 files).
(In reply to comment #12) > It is necessary as it is otherwise not possible to move files correctly (to the > scheme of the audiobook library) when the audiobook libraries folder is set > to the same folder as music library. It would also break the move in a > different folder that should happen after metadata changes (which isn't a > problem with mp3 files). Yeah, what I'm saying is that it's a bugfix for a smaller problem: let's fix that first. For example, very related to how all this is broken, is bug 689123. Audiobooks is broken that way too I think. If you add patches for those bugs first, they will be smaller patches that will also be easy to review.
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.