jacquesh / foo_openlyrics

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

Bump fb2k version requirement from v1.5 to v2.0 #286

Open jacquesh opened 11 months ago

jacquesh commented 11 months ago

This will give us access to a whole load of new functionality provided by the fb2k SDK and as per the metrics report most people are on v2.0 anyway.

regorxxx commented 8 months ago

Note due to https://github.com/jacquesh/foo_openlyrics/issues/177, many fb 1.6 users are not using openlyrics since there is no easy way to integrate the component on pure JavaScript layouts yet. And that accounts for a big portion of users.

Also some of these JavaScript components, like SMP, has not been ported to 2.0 yet. A 30% user base, per your reports is not a small portion for me. Specially having in mind that fb 1.6 user base is already smaller than it could be due to current limitations.

Just my 2cents, maybe you could run a poll or something, since reading the forums at HA most complains are essentially about lack of sources (compared to the other lyrics plugins) and no way to fetch lyrics without a hidden panel, not about other features. Metric Reports are always biased to the current user base usage, which may not be an accurate image of the potential user base (the one looking for a open source lyrics plugin alternative).

jacquesh commented 8 months ago

I thought all v1.6.x components worked on v2.0 as long as you're running the 32-bit build? Is that not the case? O_o If it is then is there much reason for people to stick to v1.6 instead of upgrading to v2.0 (32-bit)? I wouldn't say 30% is a small portion either, but keep in mind that data was collected very shortly after v2.0 was released (if we ignore beta versions) so I would expect that number to have gone down fairly sharply since then. Also The next release isn't going to happen particularly soon (almost certainly not this year, at least) so there's still more time for people to upgrade on their own before they need to worry about openlyrics.

I'm not particularly concerned about fetching lyrics without any openlyrics panels. openlyrics is specifically not a tool just for retrieving lyrics. I'd like to say I've been fairly consistent in my opinion that "if you're not using openlyrics to view the lyrics, then openlyrics isn't going to bend over backwards to help you retrieve them". We already try fairly hard to avoid making requests to websites when the results will not be used or seen. This is important to me because we are benefiting significantly from the existence of those websites and otherwise give them nothing in return so I want to try as hard as possible to be a good citizen and not generate any more traffic for them than we need to. It seems hard to maintain that if we want to support retrieval for display by other panels because we can't reason about whether the lyrics will be seen or not. Admittedly I think the main check that this would bypass is "is the openlyrics panel visible" which is true whenever you have a panel on the UI and fb2k isn't minimised. It is entirely possible that we gain nothing from that check, possibly another good use-case for other metrics (we won't have the SMP people, as you say, but that's fine because it gives us a worst-case of what we lose if we take that logic out).

The lack of new sources is a fair callout. I generally manage to find lyrics for the music I listen to so I'm not particularly motivated to add more sources, so it doesn't really happen. It's been a while though and we're accumulating a reasonable number of new-source requests on github. I've designated the v1.9 milestone as focusing on the addition of more sources :) (that version number will almost certainly get bumped at least to v1.10, because I want to put out a bugfix release soon to address #306).

regorxxx commented 8 months ago

Multiple replies to make it easier.

I thought all v1.6.x components worked on v2.0 as long as you're running the 32-bit build? Is that not the case? O_o If it is then is there much reason for people to stick to v1.6 instead of upgrading to v2.0 (32-bit)? I wouldn't say 30% is a small portion either, but keep in mind that data was collected very shortly after v2.0 was released (if we ignore beta versions) so I would expect that number to have gone down fairly sharply since then. Also The next release isn't going to happen particularly soon (almost certainly not this year, at least) so there's still more time for people to upgrade on their own before they need to worry about openlyrics. Yep, 32-bit plugins from v1.6 work fine on v2.0 as long as there are no breaking changes/bugs.

I thought your requirements about v2 were strictly requiring v2, not just porting to x64 and having extra features.

Note my approach to this discussion was the main the description of your page:

It is intended to be a replacement for LyricShowPanel3 so it is fully-featured and supports lyric searching, saving and editing directly from within foobar2000.

Taking that as aim, I considered more sources/hidden panel/compatibility to be great priority features. At least for some time... until v2 fully replaces v1.

regorxxx commented 8 months ago

I'm not particularly concerned about fetching lyrics without any openlyrics panels. openlyrics is specifically not a tool just for retrieving lyrics. I'd like to say I've been fairly consistent in my opinion that "if you're not using openlyrics to view the lyrics, then openlyrics isn't going to bend over backwards to help you retrieve them". We already try fairly hard to avoid making requests to websites when the results will not be used or seen. This is important to me because we are benefiting significantly from the existence of those websites and otherwise give them nothing in return so I want to try as hard as possible to be a good citizen and not generate any more traffic for them than we need to. It seems hard to maintain that if we want to support retrieval for display by other panels because we can't reason about whether the lyrics will be seen or not. Admittedly I think the main check that this would bypass is "is the openlyrics panel visible" which is true whenever you have a panel on the UI and fb2k isn't minimised. It is entirely possible that we gain nothing from that check, possibly another good use-case for other metrics (we won't have the SMP people, as you say, but that's fine because it gives us a worst-case of what we lose if we take that logic out). I have multiple points to give about this:

  • How big is the user base of foobar2000 + OpenLyrics (without previously downloaded lyrics) to consider hard-codding lyric retrieval to visible panels? Does it really have any impact on those servers or it's just a countermeasure for a "what if" situation?
  • This project is open-source... what stops anyone to just fork it and add that single feature? I mean, if it's not added for that "what if" situation, it is not really a barrier to implement it somehow.
  • There are other plugins allowing it, so it's not like whoever needs it, will do it.
  • There are also specific programs and scrappers for that.
  • How many users are really into installing an obscure media player, only working on windows, adding an even more obscure plugin, then having an invisible panel and automatically downloading lyrics for that? I mean, that is not real.
  • You can create such scenario with a visible foobar2000 + OpenLyrics window and batch download lyrics by scrolling through your library. There are already plugins which allow to preview tracks by some seconds. So even if your corcerns are true, they are not covered at all with the check "plugin must be visible", since that doesn't stop anyone to download lyrics with the current state of plugin.
  • That implies the next point. Panel visibility only stops people viewing downloaded lyrics with other panels, not downloading them in batch. So you are in fact targeting the wrong users with that limitation.
  • If the group of users limited by it has nothing to do with batch down-loaders, then this is just a lost opportunity. For ex. @TT-ReBORN has been pretty helpful with other developers while adding their plugins/scripts on their skin. Opening the user base opens the possibilities for better plugins and more eyes. They also created some scripts for other lyrics components btw. I'd like to say I've been fairly consistent in my opinion that "if you're not using openlyrics to view the lyrics, then openlyrics isn't going to bend over backwards to help you retrieve them".
  • One thing is viewing the lyrics IN the openlyrics panel, and onother not using openlyrics to view the lyrics. A panel is just that, a UI element. Is not totally unreasonable to allow other UI elements to display things offered by OpenLyrics; which can open a lot of functionality and collaboration with other JavaScript components or via Com+ interfaces.
  • This is one of the "problems" with sources for ex. Since Source fetching is embedded in C++, external users can not add their own sources. Check Eslyrics for ex: https://hydrogenaud.io/index.php/topic,122571.msg1034411.html#msg1034411 Source scripts can be written in JS, so it's "trivial" to add new ones without requiring a new compile of the component. (maybe something to consider in the future, along built-in ones)
regorxxx commented 8 months ago

The lack of new sources is a fair callout. I generally manage to find lyrics for the music I listen to so I'm not particularly motivated to add more sources, so it doesn't really happen. It's been a while though and we're accumulating a reasonable number of new-source requests on github. I've designated the v1.9 milestone as focusing on the addition of more sources :) (that version number will almost certainly get bumped at least to v1.10, because I want to put out a bugfix release soon to address https://github.com/jacquesh/foo_openlyrics/issues/306).

This is covered by some of the previous points; so maybe there is some kind of possibility to add external python/javscript scrappers which would totally cover any source without your intervention (even if you at some point keep adding built-in sources).

Also the existing JS scripts may help you, since the code logic at least is there.

TT-ReBORN commented 8 months ago

Hi guys,

since @regorxxx has lead my attention to this thread, I'll write my thoughts about some topics discussed here to take into consideration for future updates.

Actually I wanted to implement OpenLyrics in my theme since Lyric Show 3 isn't developed anymore, but it hindered my to do so because of two missing features. No support for fetching lyrics when the OpenLyrics panel is hidden and no lyric support when playback is radio streaming ( many of my user base are using lyrics when they listen to radio ). That is why I decided to use and implement ESLyric and it's working perfectly ever since.

I do not see any issues nor technical difficulties to make fetch lyrics happen when the OpenLyrics panel is hidden. This has already worked in Lyric Show 3 and is also working in ESLyric. I think if this would be possible, more theme developers would actually use OpenLyrics. I do not see any issues regarding traffic on servers, the foobar2000 user base is already really small and these servers can handle the requests without issues, traffic nowadays ( it isn't like 15 years ago ) also do not cost any penny.

I rarely use radio streaming myself but I would not underestimate other users that are using it. In fact in the past, I had lot of messages from users asking about radio and lyrics, meaning, always put personal preferences aside and think what the general user base would want and achieve.

ttsping, the developer of ESLyric has made a neat interface between his ESLyric component and JavaScript to be able to communicate between each other. Unfortunately the documentation is only in Chinese, but with google translate, you can take a look: https://github-com.translate.goog/ESLyric/release/wiki?_x_tr_sl=auto&_x_tr_tl=en&_x_tr_hl=en-US&_x_tr_pto=wapp It has various callbacks that make life easier for other developers and users to satisfy their need when using their own JavaScript. This took me a while to figure some things out ( never done web scrappers ) but in one week I have made 20 new lyric sources for ESLyric, all written in JavaScript and provided as a source plugin for ESLyric. You can take a look here: https://github.com/ESLyric/scripts/tree/main/searcher and also implement them in OpenLyrics yourself and improve them as you see fit. I really like this approach and applaud ttsping for making it possible because it is really flexible and every user can modify or add new lyric sources with minimal effort. I would consider this approach or something similar for OpenLyrics, because ESLyric at the current state is more open source than OpenLyrics.

I think we all know this but I write it anyways, people are mostly "hot" for having as many quality lyric sources as possible to find lyrics for their music library catalog. Also it is always the best to have as many lyric components for foobar2000 as possible and the user can decide which one to use.

jacquesh, thanks for your work on OpenLyrics.

-TT

jacquesh commented 8 months ago

This is a great discussion, but sadly is also way off-topic for this issue. That's kinda my fault though, woops. I'll touch briefly on each point here:

Regarding the retrieval of lyrics without a panel, that's #177. I've hidden some comments here as off-topic and added a summary there. I mean no offense to the authors of hidden comments, it's just to keep this thread focused. Please feel free to continue discussion of that issue there. I've also added that to the "sources-focused" v1.9 milestone so it is at least scheduled.

Regarding the "difficulty" of adding new sources: It is vanishingly unlikely that I will ever embed a python or javascript interpreter into openlyrics. If I ever embed a scripting language, it would be something smaller and easier to reason about. Probably lua. Anybody who would particularly like to see this happen is free to open an issue to track it (please do not reply here just for this, see the above note about being way-off topic >_<)

@TT-ReBORN Radio support was added a while back in v1.5 (release 2023-04-03). If it continues to not work for you (or your users) then please do open a bug report issue accordingly.


Back to the question of fb2k version requirements: @regorxxx

I thought your requirements about v2 were strictly requiring v2, not just porting to x64 and having extra features.

I don't understand. I am exactly suggesting that we strictly require fb2k v2+. My understanding is that all plugins for v1.6 continue to work on v2 (x86), so there is nothing stopping users from upgrading. I'd be interested in hearing more if this is not the case.

I have done this myself. I use Facets for the album art display and until I find something to replace it in the 64-bit build, I'm stuck on the 32-bit build so that I can continue to use Facets.