OxygenCobalt / Auxio

A simple, rational music player for android
GNU General Public License v3.0
1.8k stars 118 forks source link

Arbitrary Images for Artists #68

Open kenktk opened 2 years ago

kenktk commented 2 years ago

Describe the feature you want to implement:

Addition of rendering the artist icon in the Artists view instead of rendering album art or a collage of albums.

Is your feature request related to a problem? Please describe:

Not related to any problem unless Artist icons are supposed to be rendered already.

Why do you think this will improve everyone's usage of Auxio?

This is (IMO) a low priority feature and does not exactly improve the functionality of the application. However, it possibly makes the music library look more "complete."

Using deemix, the artist image automatically downloads as "folder.jpg" by default and is placed at the root of the artist's directory.

Due Diligence:

OxygenCobalt commented 2 years ago

I've considered this feature before, but I'm really not sure whether I would consider the feature acceptable or not.

Using deemix, the artist image automatically downloads as "folder.jpg" by default and is placed at the root of the artist's directory.

Scoped storage limitations prevent Auxio from walking the file tree directly. So, I have three options here:

  1. Allow the user to set the artist image manually. This would respect scoped storage, at the cost of more work for the user and a more cluttered artist menu.
  2. Automatically fetch artist images from last.fm or something else. This is very convenient, but also a non-starter, as it would violate the no-connectivity promise of Auxio.
  3. Do some MediaStore black magic and try to get detect artist images that come from the correct path. This would be convenient, but also suffer from frustrating bugs since MediaStore is a load of crap and weird directory structures could easily result in incorrect images.
    • If I am lucky, this process could even not be bound to the main loading process. Image loading would just wait until the query is done and then use the artist images. That would eliminate the loading time concerns.
    • Given the instability and niche nature of this system, I'll likely gate this behind an option so that most users can use the more reliable mosaic image.

Honestly, the "easiest" implementation would be 1, but I'm gravitating towards 3 as the behavior would be much more sensible. I'll keep this open as I can definitely see the benefit of it, but I want to focus on other issues first.

kenktk commented 2 years ago

Honestly, the "easiest" implementation would be 1, but I'm gravitating towards 3 as the behavior would be much more sensible. I'll keep this open as I can definitely see the benefit of it, but I want to focus on other issues first.

I definitely agree with this. 3 would be the most professional method of doing this however, from the looks of it, the implementation could be interesting... lol. Without a doubt a low priority enhancement. Bigger fish to fry right now.

OxygenCobalt commented 2 years ago

Bigger fish to fry right now.

Not exactly, I'm planning to try my hand at #66 [among other issues] for the next version, so I could probably bundle this issue with the other artist improvements I'm wanting to implement. also it gives me an excuse to continue procrastinating with playlists but lets not talk about that

OxygenCobalt commented 2 years ago

Quick question @kenktk: Do you/library manager programs truncate the folder names for each artist if they're too long? I'm trying to detect when Auxio has accidentally picked a self-titled album [i.e a path like <artist>/<self-titled-album>] and I'll need to match artist names to fix it. The issue with that is that it may result in unintended behavior if artist names are truncated in any way, so I want to get feedback from someone with a well-tagged library before I continue.

kenktk commented 2 years ago

I haven't noticed any truncating yet. I have some pretty long titles and artist names in here (90+ chars). All of my songs have album, artist, and song name tags with them. I am not sure if I have ever encountered Auxo trying to fill any of those tags with a path location.

OxygenCobalt commented 2 years ago

Okay. That means that I should have a viable artist image implementation. I'll try to integrate it eventually.

illdeletethis commented 2 years ago

i"d absolutely hate having the app automatically download pictures to display, be it for artists, albums or anything else another music app i tried (shuttle) did that until disabling it in settings that app has an option for manually setting artist pictures, which is good, but if none are set there"s just a blank image the solution here with the multiple album artwork as a placeholder for artists is probably the best possible option as long as no artist image is set

OxygenCobalt commented 2 years ago

i"d absolutely hate having the app automatically download pictures to display, be it for artists, albums or anything else

I agree, it's quite obnoxious when the app decides that it should download the single cover for a song because it downloads images from the internet, not to mention the accidental collision issues I've seen on other apps.

that app has an option for manually setting artist pictures

Personally, I don't like the idea of manually setting artist pictures, as it clutters the artist menu somewhat. I would rather prefer an opinionated system that automatically traverses the image database to find artist images, which is currently what I'm going for. That way, you still have control over what artist images you want [or if you even want them in the first place].

OxygenCobalt commented 2 years ago

If you are wondering about the hold-up with this feature. I really want to tackle Auxio's technical debt [See #72] before I move on with new features.

OxygenCobalt commented 1 year ago

Sorry to necro this, but I'm not sure how to make this feature work in light of #195. Is there any convention for how the "folder.jpg" technique works with multiple artists @kenktk @illdeletethis?

This is what I could consider, assuming an album with artists "Foo" and "Bar" that are of equal importance:

  1. Two folders called Foo and Bar that contains folder.jpg instances. Only one contains the album, and the other is either empty or has a symlink to the album. I think this is the most logical system.
  2. A single folder called Foo_Bar that contains a folder.jpg unique to that combination. Auxio couldn't handle this, as it will split up Foo and Bar into separate entries with #195.
  3. A single folder called Foo_Bar that contains the album and two images, one named Foo.jpg and the other named Bar.jpg. This is easy to handle, but I doubt it's the convention.

P.S: The reason I'm not really working on this is that it involves more global mutable concurrent state. I am kind of sick of working on such after the past few versions.

ghost commented 1 year ago

Would it perhaps be more flexible to implement a way to import user-created "cover packs", much like there is an option to import icon packs in Aegis? It would take away from having to implement a system that constantly scans for images, while also not having to guess the folder structure of audio files.

The downside is that actually creating the pack for your collection might be a bit tedious, depending on the defined structure for these (though, I guess it wouldn't be too hard to implement a cmd tool which can do this automatically, sourcing from an API like Deezer for example).

OxygenCobalt commented 1 year ago

I don't know. Way too many people leverage the folder structure for me to ignore them entirely, or add some extra step to generate one. Neither can I contact an external API like Deezer due to my promise to never have internet connectivity.

OxygenCobalt commented 1 year ago

I think I've found a resolution for this and #104. Basically, I'm just going to blindly query the media database and check for two things:

  1. The file is named after an artist entry, with leeway for underscores and lowercase values (ex. 65daysofstatic.jpg or ellen_allien.jpg)
  2. The file is named with some generic value (artist.jpg, folder.jpg) and it's direct parent folder matches an artist entry.

This may pick up some false positive artist images, but that's simply the cost of making a reasonable system.

OxygenCobalt commented 1 year ago

Another idea is that I could extract specific APIC/PICTURE tags with the "Artist/performer" type. This is a lot simpler technically than traversing the media database, but is also far less common. I might do this to implement artist images as an MVP, and then roll artist images based on folder structure into #104.

illdeletethis commented 1 year ago

Both your initial solutions sound good option 1 is the one that less messes with picture gallery apps, as everything could be in just one folder, maybe one that could be chosen in auxio the same way music folders can be whitelisted. option 2 seems to be quite common for desktop use though, and works well without users having to move pictures or whitelist anything.

With option 3, does that mean the picture is tagged to the audio file, so no additional file, meaning least messing up of gallery?

OxygenCobalt commented 1 year ago

You are right about all of those @illdeletethis.

OxygenCobalt commented 1 year ago

I've managed to throw together a pretty basic artist image system based on the picture tag option. It supports multiple artists at once. Here's a demo of it below:

https://user-images.githubusercontent.com/65370175/192674607-a8a6db1d-ae45-457e-a3f8-a8a3c30fa3f4.mp4

If you want to know how to set up your image tags so that Auxio will create artist images from it, this is the tag setup as shown in kid3:

image

Parsing artist images from an obscure quirk in the ID3v2/FLAC specification is definitely not the most "sensible" solution, but it's also the solution I can quickly bodge into the current app architecture, This allows me to implement Artist Images (Something I've wanted in Auxio for awhile) immediately and benefit my own usage.

I'm still planning to add filesystem-derived artist images (Just like #104), but I think that will require a much broader rework to the music loader. To describe it non-technically, I need to flatten both types of images so that they can both be interchangeably used in-app. The specifics though are difficult and annoying and are likely something that will be pushed far into the future.

OxygenCobalt commented 1 year ago

Okay, I've discovered a minor limitation with the tag-based system: Taggers have awful support for adding non-cover art pictures. Kid3 can add them, but then has bugs when trying to edit them. Picard has no support for non-cover art at all. I'm not sure about Beets, but I'd imagine it also does not support it either. Since the tag-based system is an MVP largely for my own use though, I'll tolerate it.

illdeletethis commented 1 year ago

lovely seeing this, thank you for the work you put into this app. really looking forward to the next updates

OxygenCobalt commented 1 year ago

Honestly, I'm going to delay implementing this. The tagger issue is just far too annoying for me to find it sensible. I think once I get a good MVP of my personal tagger project going I'll re-visit it, as I'm explicitly designing it to handle the picture format I desire.

OxygenCobalt commented 1 year ago

Okay, this is now blocked by #322, which should enable directory-based images in a speedy manner.