Does your device actually let you browse it by folder?  I have vision W and a zen vplus, and afaict neither let you do that - they automatically generate lists of genres/albums/artists.  That's why I didn't worry about putting tracks in folders other than the Music folder.

The Creative Zen players do not allow you to browse by folder from the device itself. I believe the Mieszko is referring to mounting the device for viewing using gphoto tools or in Windows explorer without Banshee. If you're only using Banshee to manage the device, then it the internal folder structure shouldn't matter to the end user.

Yes, and also when mounting it with mtpfs. It matters, because it severly slows down browsing, when I have 4000 tracks in one folder. I also want to be able to easy manage music "by hand" when necessary.

I have the same issue with both Creative Zen Vision: M and Creative Zen Touch.

By the way, I don't think it is just a matter of internal structure because :
1/ as already said it takes ages to browse and update
2/ file names are wrong ; there are composed only of track number + track title which can lead to crushing of files

Bulk changing the assignee to banshee-maint@gnome.bugs to make it easier for people to get updated on all banshee bugs by following that address.  It's usually quite apparent who is working on a given bug by the comments and/or patches attached.

My MTP also allows browsing per directory (and actually, since display and buttons are small, it's my preferred way of browsing the songs; everything else takes ages). And yes, being able to manage files in a file browser is a plus, too.

Created attachment 229307 [details] [review]
Patch to add nested folder support for MTP storing and retrieval

I've had problems with tracks synced from other players being counted as "Otehr" and not showing up in the track list. It appears to be because Banshee quite literally says "file is in Music folder" to count it as a music file. The attached patch may not be the cleanest, but it works for me to get nested folder support so that a) Banshee stores files in nested folders on the device (/Music/artist/album/track.ext) and b) Banshee loads *all* music files from anywhere under /Music/.

The patch could possibly be a bit more efficient as well, but the existing code doesn't seem to cache many objects, so I've avoided caching and keeping parent/child objects as well, even if it takes a slight performance hit.

If anyone is using openSUSE 12.x then the patch (along with a patch to handle changes in libmtp) will be built into my version of the latest packages in the OBS: https://build.opensuse.org/package/show?package=banshee&project=home%3AIBBoard%3Adesktop

Created attachment 232825 [details] [review]
Properly formatted patch for nested folder support on MTP

Hi, I'll give your patch a try, probably over the weekend. We shall worry about efficiency later ;)

Just a quick thought, when loading from the device, did you consider evaluating files based on their type as a possible solution?

I kept the loading logic as close to the original as possible for ease of implementation. I've not got anything against dedicated "Music" folders, so I just worked on organising it below that level. It could probably be done based on type, though.

Created attachment 268512 [details] [review]
Adds folder heirarchy support to Mtp track detection

Hi, nice work!

I was asked to try and separate the patch into read and write support by one of the banshee developers to try and make it easier to back port for older versions. 

I briefly tested the write support which worked well, but I've concentrated on the read side to start with. I did a little massaging:

- Changed InFolderOrSubfolder to an overload of InFolder accepting a recursion boolean, which is then called from InFolder. Just prevents the InFolder condition being in there twice.

- I removed the MtpDevice argument from the Track.InFolderOrSubfolder as we (should!) have the device reference in a member variable already. As you probably did, I noticed that can be null so I modified the Track constructor to at least always expect one. It might be worth throwing in the constructor if the user passes a null reference but I didn't go that far.

- In Folder.Find(MtpDevice, uint) I thought it makes more sense to throw if the user passes a null MtpDevice reference instead of returning false. Also, I couldn't see any uses of the null object pattern in close proximity so decided it makes more sense to return null in the case where we don't find the folder. Last, I wrapped the section after getting the folder list in a try/finally so that we can ensure the unmanaged folder is cleaned up after we're done.

- To add some protection I did a little error checking in HasAncestor, making sure the folder's not null and throwing if so, in addition to making sure we're not trying to compare two folders from different devices.

Ok with you?

Thanks.

Created attachment 268518 [details] [review]
Write support

I separated the write support into this patch, and gave it a slight massage in line with what we see in MassStorageSource.cs.

I took the liberty of adding similar support for Podcasts, even if a device happens to have a null PodcastFolder member, as in my Samsung S2.

This ok?

Nice work Nicholas!

As IBBoard was the first to implement this patch, I hope you don't mind that when I review and commit these patches, I'll mark one of them with IBBoard as the author.

(In reply to comment #12)
> I took the liberty of adding similar support for Podcasts, even if a device
> happens to have a null PodcastFolder member, as in my Samsung S2.

Shouldn't this logic maybe be part of the Podcast extension? Just wondering.

My code was a quick hack that worked well enough for me. I was aware there was some error handling and conditions that I probably wasn't handling, but I didn't know MTP enough to cover it all. I thought there might be some optimisations as well, but I wasn't sure how much the native references would change, so I didn't know how safe it was and went for "too much processing is better than crashes through optimisation". I don't mind if Nicholas is the author and I just get a "based on initial patch from IBBoard" note in the commit (if you guys do that).

SGTM :)

I'll review the patches soon (probably mid-next-week).

knocte: Of course I don't mind. It was really nice to find one thing on the
list
already done!

I haven't looked at the Podcast extension, but the MassStorageSource seems to
generate its own paths. I'll make a note to take a look and see what's in there
:)

I did have a look at categorizing files by type when reading, but this approach
is pretty quick and won't miss much. Also, the file type recognition in the
Mtp.Track class is a little primitive for the moment so a standard way of doing
this over a file's uri would be handy, although I don't think it exists yet.

IBBoard: I barely touched your patch apart from splitting it and adding a bit
of error checking. Most of my time was spent trying to get my copy of
monodevelop to write spaces instead of tabs. ;)

Created attachment 268592 [details]
Caching Test Run

I decided to do a little more testing and look deeper at the MtpSource.

Noticing that we have a Dictionary<string, Mtp.Album> I decided to expand on that a little by turning it into a Dictionary<string, Tuple<Mtp.Folder, Mtp.Album>> with the patch attached to the next post.

The results are presented in the text file I attach here, I've worked out some simple statistics in the "Summary" section from which it looks like caching saves us some time, at least in this fairly contrived example ;)

Created attachment 268594 [details] [review]
Adds caching and compresses for loops with a Predicate

I couldn't obsolete the Write support patch and "git format-patch HEAD~3" just gives me three patch files, number 2 was just to pull the latest Hyena, hence the 0003 in this patch number. If applied straight after "Write support" then it should work correctly ;)

Comment on attachment 268512 [details] [review]
Adds folder heirarchy support to Mtp track detection

I committed this to master and stable branch. Thanks guys!

Comment on attachment 232825 [details] [review]
Properly formatted patch for nested folder support on MTP

Marking this one as obsolete, since Nicholas has split it in two.

*** Bug 722991 has been marked as a duplicate of this bug. ***

Thanks to Nicholas' work on splitting the patches, FYI his patch was committed as solving bug 724678, which is a subset of what this bug was about.

Now that only the write-part of this bug is meant to be discussed here, I think we can all agree that this is an enhancement, so I'm marking it as such.

Hi,

I'vereported this issue nearly 6 years ago, now I am neither using banshee, nor my ZEN Vision:M.
I can not remove myself from CC list (probably because I'm reporter of this bug).

Can someone please make bugzilla not e-mail me about this bug anymore?

Cheers,
Mieszko

(In reply to comment #23)
> I'vereported this issue nearly 6 years ago, now I am neither using banshee, nor
> my ZEN Vision:M.
> I can not remove myself from CC list (probably because I'm reporter of this
> bug).
> 
> Can someone please make bugzilla not e-mail me about this bug anymore?

I don't think that's possible (unless maybe you contact the sysadmin team, they're usually active in IRC). I recommend creating a bug in bugzilla's bugilla (bugzilla.mozilla.org) also to request this feature request :)

In the meantime, the easiest solution could be to just create an email filter in your end, or unsubscribe altogether from all bugzilla's email if you don't use any of gnome either.

Thanks

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.