Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow youtube connector to get data from Youtube Music #3526

Merged
merged 5 commits into from
Mar 12, 2023

Conversation

Boelensman1
Copy link
Contributor

@Boelensman1 Boelensman1 commented Dec 18, 2022

Describe the changes you made
Add a track info getter that uses youtube music to get its information. This information seems to be more reliable than the info in the description. Downside is that this getter has to make a request, which takes a bit, so there's a bit more delay before the scrobbler finds the information. It does cache all requests it has made already, so this is only the first time, as long as the user doesn't restart their browser.

At least in my experience this getter seems to me more reliable than the ones based on description/title, but I'm not sure how to verify this as it might depend greatly on the type of music you listen to. It's not perfect regrettably, for example it might swap names around compared to the most common value in Last.fm, eg. "Fireboy DML & Asake" becomes "Asake & Fireboy DML".

I also added an option to only scrobble videos that are recognised by yt music as a music video, which seems to work quite well.

I can only write in English so someone else will have to translate this option to the other locales.

This option allows you to only scroble music recognised by youtube music as a music video.

Only added locales for english.
@yayuyokitano
Copy link
Member

Looks interesting. I can see a few potential issues, but I need to spin it up to confirm, I think I should have a bit of time tomorrow.
The addition of a request shouldn't be a massive issue I think, we already have a couple of connectors that do this.
Given that this is a pretty substantial change to the most used connector I'd definitely like for other organization members to weigh in before it is merged.

I can also see that this getter should definitely be disableable in settings.
For instance, youtube music tags will usually be a regression for those that prefer scrobbling untranslated titles without having their youtube/ytm interface actually be in that language.

@Boelensman1
Copy link
Contributor Author

Boelensman1 commented Dec 18, 2022

As requested, added an option to disable the getter, it works independently from the option to only scrobble if yt music detects it as a music video. So the behaviour is as follows:

Both options enabled or only the getter enabled:
Getter makes request and runs as usual, returns artist & track (if found)

Only 'scrobbleMusicRecognisedOnly' enabled:
Getter makes request to fill the cache to allow the check on if youtube music thinks this is music but returns an empty object so next getter is run

Neither option is enabled:
Getter immediately returns empty object, no request is made.

Specifically the getter that uses youtube music, when musicVideoType is undefined, no error will be thrown and a generic catch is added.
@yayuyokitano
Copy link
Member

Hi, do you want to add some of the extra logic that you added later on added to this PR before it is merged, or do you want it to be merged as is?

@Boelensman1
Copy link
Contributor Author

I can't really add the extra logic as that's all about returning multiple possibilities for matches which isn't useful for this project (yet?), so it can be merged as is!

@yayuyokitano yayuyokitano added connector This issue or pull request is related to connectors new-feature For PRs that add new functionality minor-change For minor changes labels Mar 12, 2023
@yayuyokitano yayuyokitano merged commit 464abc4 into web-scrobbler:master Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector This issue or pull request is related to connectors minor-change For minor changes new-feature For PRs that add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants