museeks icon indicating copy to clipboard operation
museeks copied to clipboard

Feature/edit more id3 tags

Open igorer88 opened this issue 1 year ago • 8 comments

This PR helps to solve #732

  • Added some basic metadata missing fields to edit track field.
  • Fix artist description label that was on title field.

igorer88 avatar Sep 30 '24 23:09 igorer88

Ok, I'll start with your comments on #732:

  • I fixed the DB imports, rebasing on master should make things work now

The cover is present on the Other tag, while Museeks only searches on the Front Cover tag.

If we can scan for the cover in some kind of priority (Front Cover first, then Other, then then any cover), that would sounds good to me.

About storing the cover on the tag, well that's the standard way, it means that it would work on every other music player.

Ideally, it would be great to have the option to do both,. The reason I am a bit reticent for that is I am afraid of messing someone's library files, and that's why placing a cover.jpg in their folder seems safer, even though it's not x-player compatible. I am starting why some players copy all files to another location before changing them, but I would like to avoid doing that.

Assuming a 512x512 should be <100KB, it's probably ok to embed them into the file itself. As the cover is not set in the DB anyway, I would do that separately from this PR though:

  • 1 PR for more fields udpate
  • 1 PR for optionally edit ID3 tags
  • 1 PR for setting the cover as ID3

martpie avatar Oct 01 '24 11:10 martpie

Thank you for the update, could you squash your commits and make sure master is the base branch? merge commits make it really hard to review as a the commits from master are duplicated.

martpie avatar Oct 05 '24 23:10 martpie

Ok, I did a rebase and I think I broke it further. hahahha

imagen

I did a bun install But it' s still missing that macro error.

imagen

igorer88 avatar Oct 06 '24 00:10 igorer88

I'm starting to think that I should do a new fork hahahaha

igorer88 avatar Oct 06 '24 00:10 igorer88

I did a bun install But it' s still missing that macro error.

Try cargo Install --path src-tauri from the root of the repo? And make sure you got the latest version of rustup setup?

martpie avatar Oct 06 '24 00:10 martpie

Thank you for the update, could you squash your commits and make sure master is the base branch? merge commits make it really hard to review as a the commits from master are duplicated.

I've never done commit squash before, so let me investigate how to do it.

igorer88 avatar Oct 06 '24 00:10 igorer88

https://www.git-tower.com/learn/git/faq/git-squash is a good resource :)

martpie avatar Oct 06 '24 10:10 martpie

If you use Github Desktop, it's fairly simple too:

Screenshot 2024-10-06 at 12 46 36

Then I guess, fetch all, rebase from mater, and hopefully this shoud work. Force pushing to this branch is ok, or you can play with another branch to be safe.

martpie avatar Oct 06 '24 10:10 martpie

Hi! @martpie sorry, I've been busy lately, had to travel and stuff. I'll do the squash and then I'll do another rebase for the latest updates.

igorer88 avatar Oct 29 '24 02:10 igorer88

No problem, I'm still stuck on a couple of other issues (migrating to SQLite + file associations), I will probably not release a new version of the app until they're figured out

martpie avatar Oct 29 '24 10:10 martpie

hi! @martpie, the master branch is broken again? or maybe I need to check the last merge...

imagen

igorer88 avatar Nov 29 '24 05:11 igorer88

Master should be passing(CI is), make sure you rebase and follow the setup instructions (bun install, bun tauri dev etc). Eventually, cargo test too if you are changing some types.

I see there is a conflict in your branch for the lockfile, maybe delete the file and run bun tauri dev again?

martpie avatar Nov 29 '24 10:11 martpie

btw, are you using Limux Mac or Windows?

martpie avatar Nov 29 '24 10:11 martpie

Mac

igorer88 avatar Nov 29 '24 12:11 igorer88

Let me try to wrap this up, I'm finalising 0.20.0, and I'm doing a lot of chore/code revamps introducing a lot of conflicts. I'll try to squash all these commits with you as author and finalize the feature myself.

martpie avatar Dec 04 '24 17:12 martpie