jacquesh / foo_openlyrics

An open-source lyric display panel for foobar2000
MIT License
429 stars 24 forks source link

Feature suggestion: Better handling of instrumental auto-edit #129

Closed HeresJonny closed 11 months ago

HeresJonny commented 2 years ago

Now that I finally found the "instrumental auto-edit" this doesn't seem to be exactly what I would like. The auto-edit seems to simply remove the lyrics tag completely. But what happens if the track is played again, won't the same undesired content be added to the tag again?

I would prefer if that auto-edit could optionally fully replace the lyrics tag with user-definable content like [00:00.00]- Instrumental -

At this point in time, the auto-edit only seems to be applicable to the currently playing track. It would be great if some kind of batch-edit would be possible for several selected tracks.

elandorr commented 2 years ago

Funny you post this just when I was about to. I fully agree - I always manually edit instrumental to prevent any searches. I try not to use online searches in the first place, but this is useful either way: a clear indication. Did I not tag that one because it is an instrumental, or is it untagged?

I see foo_openlyrics currently saves an exception to the database: https://github.com/jacquesh/foo_openlyrics/blob/3bd5479e2ae81deb64c2fabb68e66d33c859c2bd/src/metadb_index_search_avoidance.cpp#L132 I would like to avoid that as well. Any unasked for db-writes. My collection is somewhat huge and if there's no explicit need to store something, it shouldn't be. As far as I know, there is no way to (selectively) clean the fb2k db.

batch-edit

A great idea. I selected the relevant pieces and then added the lyrics tag myself in the Alt-Ret menu, but it'd be faster directly supported.

jacquesh commented 2 years ago

But what happens if the track is played again, won't the same undesired content be added to the tag again?

No. OpenLyrics maintains a database of tracks that do not have lyrics. This lets us avoid searching every time on tracks that have repeatedly failed in the past. The "Mark as instrumental" edit now just adds the track to that list. This is more consistent because now you don't have some tracks with lyrics saying "Instrumental" and others that simply do not show any lyrics at all, they just all have no lyrics (as if you had done nothing and the search had simply never returned any results).

Any unasked for db-writes.

There is no option for disabling the previously-existing search avoidance that happens after several failed searches. This is intentional and it prevents unnecessary searches. So the remaining comparison is between the auto-edit that creates a lyric saying "Instrumental" and the auto-edit that updates the search avoidance db. Neither are unasked-for and both write to disk somewhere. I'm not convinced the two are materially different in this regard.

As far as I know, there is no way to (selectively) clean the fb2k db.

I haven't looked into it but I suspect this is something we could add if necessary. If this is something you want then please create an issue for just that feature (but if you do then please substantiate why it would be helpful for you).

It would be great if some kind of batch-edit would be possible for several selected tracks.

Batching auto-edits from the playlist selection context menu (same as the existing "Show lyrics" and "Search for lyrics" buttons) is something I'd consider adding. As above, if thats something you would find useful then please create an issue for just that feature.

elandorr commented 2 years ago

I really don't have time for fun github posting, but maintaining the collection is kind of a big deal to me, let's go!

Let me convince you: The auto-edit is not 'auto' as in 'unasked for'. You context-click -> edit -> instrumental, you take the action. We were never told it'd write anything with automatic searches.

The db does not prevent searches for good or supports the collector - it's not portable, and lasts only as long as the current infinitely bloating install. Eventually you must reset it one way or another. Or Microsoft forces spyware-account-only aaS like with Windows 11, preventing your continued use of fb2k, undoing the effort. A clean collection with proper tags can be hashed, backed up, and will endure. It'll work with a self-made CLI tool, just as on DOS and Haiku. Reading tags is portable (Even if tag internals are annoying to handle, but that's another topic.). To save a few MB per TB, one could write an empty tag (if the spec allows, can't recall ad hoc), or only i.

Add what I posted already:

this is useful either way: a clear indication. Did I not tag that one because it is an instrumental, or is it untagged?

At best, the internal db is expensive temporary lazy-storage for no real benefit. A tag can be maintained and lasts. I'd argue the kind of user to set up fb2k is one that cares, or he would go for trendier options that require 0 interaction, like an iPhony. The typical user is slave to 'streaming' aaS now anyhow.

I'd like to avoid any unasked-for db/storage/net interaction. I get that fb2k itself is not open source to begin with, but there's some weird misunderstanding in the fb2k world between what constitutes my computer, and what yours. A collection is a lot of effort. (Too much, admittedly. I'm trying to reduce!) and you cannot just infinitely dump in a db that will never be known/seen/cared for. There's not even any documentation outside the SDK. Startup and query are already quite heavy, and I don't even have the full collection added currently. Last year, I only just found out how many plugins write infinitely much for no good reason.

I wish fb2k were simply FOSS, so we could recycle that db altogether and create a proper pg backend for large collections. And an interface that clearly shows users what it holds. I considered doing that externally and plugging it into fb2k, so it's just a player, but I don't know how far the SDK will let you in, or how long it itself will survive. Not worth it for closed source. I recently went through the entire setup, and virtually all of it is unmaintained, most often without source. It doesn't take much to brick it. There's not a lot of interest or care in the 'community' - some of those HA threads of great add-ons I use only accumulated a handful of pages in 15 (!) years. I think fatigue set in, that's very understandable with the state of the world. I still care enough to type long comments, apparently.

I'm very much pro-lightweight in everything, but you go beyond with your fear of traffic. The very page you're viewing right now is 2MB raw, 605KB compressed, and I have a huge blacklist, so it's probably more without. Consider typical modern song structure, and you get maybe 800B in fat unicode. (+ HTTP headers which could be bigger) Let's make that 2KB for today's understanding of 'long form poetry'. Let's say it's spoken word gabber style and done in just 3mins each. At 10 hours a day non-stop, that's 400KB. Sending this comment eats four times more.

To avoid unnecessary searches, I'd focus on perfecting manual entry: bulk search all available lyrics for a CD, add more sources or allow adding custom ones with higher hit rates, allow efficient picking and editing. The better the entry experience, the more users will use it. (The manual trigger currently searches all sources, which also adds useless requests, as e.g. that Chinese thingy is useless. No issue having a few bytes more, but since you speak of unnecessary searches.) Once saved in the tag, you're done forever. Use fb2k's layout tool to arrange it properly, and it won't get any more efficient. (Except my custom large scale db idea, but at that point we're optimizing.)

Another easy and obvious fix you may already have implemented: Don't search when minimized. fb2k will be in background 99% of your day.

jacquesh commented 2 years ago

I really don't have time for fun github posting

Agreed, so rather than responding each of your many points with my own arguments, let me simplify by going straight to the conclusions:

  1. It is still entirely possible for you to manually add lyrics saying "Instrumental" or whatever you like, its just less convenient. I've added #133 to speed this up.
  2. I am not going to remove search avoidance (at least not with the sorts of arguments presented here)
  3. You might be able to convince me to add a separate auto-edit that adds a preset lyric saying "[Instrumental]" or whatever, as the previous one did. However I would prefer not to because...
  4. I agree that manual entry can and should be improved. Please do create separate issues for any specific improvements you'd like to see on that front.
  5. "Bulk search all available lyrics" is already possible. Select any number of tracks/albums/artists, right click and select "Download lyrics")
  6. If you want more sources, you are welcome to create a PR for them. Failing that, there are already some issues for adding specific sources and you can create new issues if you want a source that you don't see already requested.

I think we have digressed somewhat from the issue @HeresJonny originally posted. I claim that if #133 were implemented then it would take about the same amount of time and effort to manually add a lyric saying "Instrumental" (or whatever custom text you'd like) that it does to go rightclick -> Auto-edit -> Mark as instrumental. If we expanded #113 to include auto-edits that might help, as would #134 that I've just added.

H2Swine commented 1 year ago

I don't know if I should raise a new issue (there are enough of those too), but: Shouldn't the message "Auto-skipped because it failed too many times" disappear when I have marked a track as instrumental?

(... by the way, is there a way to search up those auto-skipped ones and retry them?)

jacquesh commented 1 year ago

@H2Swine There is currently no (easy) way to enumerate all auto-skipped tracks. I'm not sure what value it would add because until you play a track, the presence (or lack) of lyrics is largely irrelevant, and you'll see as soon as you play the track. Off the top of my head I'm not 100% sure whether bulksearch will skip due to the autoskip or not, but if not then you could always bulk-search for an album (or whatever) when you find missing tracks. In general it should be pretty rare that lyrics were not available for long enough that it started autoskipping but now suddenly are available.

Autoskip does not currently distinguish between tracks that are autoskipped because no lyrics were available, and tracks explicitly marked as instrumental. Its definitely something I've thought about doing (possibly as an extension of #57). I don't think there's an existing ticket about it, so feel free to create one. It may well be that there isn't a way to reliably distinguish between the two in hindsight though and if so, tracks marked as instrumental would continue to show the "we didn't find any lyrics" message. Not necessarily a deal-breaker (it'd still be an improvement) but it reduces the immediate value of the feature slightly.