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

[Feature Request]Can MPV get_property add media-title choice. #169

Open
AlickH opened this issue Dec 25, 2021 · 22 comments
Open

[Feature Request]Can MPV get_property add media-title choice. #169

AlickH opened this issue Dec 25, 2021 · 22 comments
Labels
enhancement New feature or request

Comments

@AlickH
Copy link

AlickH commented Dec 25, 2021

Sometimes the url stream link did not contain the file name, but mpv can set the media title through m3u list or by --force-media-title command.

@AlickH AlickH changed the title [Request]Can MPV get_property add media-title choice. [Feature Request]Can MPV get_property add media-title choice. Dec 25, 2021
@AlickH AlickH changed the title [Feature Request]Can MPV get_property add media-title choice. [Feature Request]Can MPV get_property add media-title choice. Dec 25, 2021
@iamkroot
Copy link
Owner

iamkroot commented Dec 26, 2021

Yeah, we probably could do it instead of reading the path.
But the issue is deciding between the two. I've found that many a time, the Media Title does not contain meaningful information (eg: Only the episode title, but no show name). We can't simply stop using the path for extracting info and switch to title.
And in cases where both contain partial info, one of which is wrong (eg: Weird url from which we somehow extracted some gibberish, but valid info - remember, this is all powered by regexes at the core), how do we decide what to use?

This was my original motivation for choosing path instead of title, since that is something that the user could control (for any media player). But now that we have streaming support from the web, this assumption doesn't hold true. However, I'm not sure on how to fix this without requiring a lot of manual intervention.

Some possible solutions-

  1. (Easy) Prefer Title instead of path when the media is being streamed from a url. This will again run into the same issue of the title property not having enough info very frequently.
  2. (Hard) Do media info extraction on both, the path and title. Then, come up with some scheme to merge the results from both.
    1. What to do with conflicts? Maybe we can have it set to prefer path over title, or vice versa.
    2. Even better, this preference could be configured per whitelist entry. Since we would usually prefer to use the url/path for some good media sources, and the title property for the cases when the url isn't helping.

@AlickH
Copy link
Author

AlickH commented Dec 26, 2021

Maybe could combine this with whitelist to choose which path or url to use path or title, or let user to write it to config file to choose title or path, or popup a dialog box to choose which to use when there are two options.

@iamkroot
Copy link
Owner

popup a dialog box to choose which to use when there are two options.

That is way too intrusive. I would like the scrobbler to not require manual intervention, especially on every file.

let user to write it to config file to choose title or path

Yeah, this seems feasible. Will try implementing it when I get some free time.

@iamkroot iamkroot added the enhancement New feature or request label Dec 28, 2021
@umop3plsdn
Copy link

umop3plsdn commented Oct 27, 2022

I came to ask about this as well. Apparently when i'm streaming some content it only picks up filename from the IPC socket for scrobble instead of media-title

@iamkroot
Copy link
Owner

As stated above, my main worry with this is making the configuration too manual. But now that #214 has been implemented, we have a pretty powerful way of specifying how to remap raw media info.
Currently, the feature only works on info extracted from file paths, but it should be relatively easy to extend that to file title retrieved from mpv. Will try to work on this soon.

iamkroot added a commit that referenced this issue Nov 26, 2022
We give priority to title if it is present in title_whitelist.

Closes #169
@iamkroot
Copy link
Owner

iamkroot commented Nov 26, 2022

I have implemented an initial version of this feature in the title-extract branch.

If the title provided by the media player is present in fileinfo.title_whitelist sequence, it will be used to extract info instead of file path. There is currently no way to merge the infos extracted from title and path individually.

Install from branch:

  1. Stop the scrobbler with trakts stop
  2. Run pipx install --force --pip-args='--force-reinstall' git+https://github.com/iamkroot/trakt-scrobbler.git@title-extract
  3. Start scrobbler with trakts start

Configuration: trakts config set --add fileinfo.title_whitelist "Some Name"

Now, any media that contains "Some Name" in the title will use that title for metadata.

Please give it a try and let me know if everything is working as expected.

iamkroot added a commit that referenced this issue Dec 30, 2022
We give priority to title if it is present in title_whitelist.

Closes #169
@iamkroot
Copy link
Owner

iamkroot commented Jan 6, 2023

@AlickH @umop3plsdn would appreciate your feedback on the design!

@umop3plsdn
Copy link

umop3plsdn commented Jan 12, 2023

Sorry didn't see this until today.

@umop3plsdn
Copy link

2023-01-12 09:12:32,067 - WARNING - scrobbler - trakt_interface - Invalid trakt id for 1649804033
2023-01-12 09:12:32,067 - ERROR - scrobbler - init - Unhandled exception
Traceback (most recent call last):
File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
self.run()
File "/home/da5id/.local/pipx/venvs/trakt-scrobbler/lib/python3.10/site-packages/trakt_scrobbler/scrobbler.py", line 36, in run
self.scrobble(verb, data)
File "/home/da5id/.local/pipx/venvs/trakt-scrobbler/lib/python3.10/site-packages/trakt_scrobbler/scrobbler.py", line 78, in scrobble
resp = trakt.scrobble(verb, **data)
File "/home/da5id/.local/pipx/venvs/trakt-scrobbler/lib/python3.10/site-packages/trakt_scrobbler/trakt_interface.py", line 48, in wrapped
verb, params = next(coro)
StopIteration

@umop3plsdn
Copy link

also its still picking up the same thing:
trakts test mpv
Testing connection with mpv.
Please ensure that mpv is playing some media.

  • Connected
  • Got info
    Playing 1649804033 S10E07 at 55.88%

@umop3plsdn
Copy link

echo '{"command": ["get_property", "media-title"]}' |socat - /tmp/mpvsocket
{"data":"bleach-episode-7-dub","request_id":0,"error":"success"}

iamkroot added a commit that referenced this issue Jan 12, 2023
We give priority to title if it is present in title_whitelist.

Closes #169
@iamkroot
Copy link
Owner

Sorry, that was a small bug in the code. I've pushed a fix. Please re-install using the instructions above.

@umop3plsdn
Copy link

umop3plsdn commented Jan 13, 2023

alright reinstalled and it looks like its still pulling off the same location. for the record i double checked that the changes you pushed out were actually present in /home/da5id/.local/pipx/venvs/trakt-scrobbler/lib/python3.10/site-packages/trakt_scrobbler/player_monitors/mpv.py just to double check since it was the same result. Also, to help i tried a youtube vid randomly https://www.youtube.com/watch?v=djA0fYk65yI
trakt-scrobbler on  title-extract via 🐍 v3.10.9
➜ echo '{"command": ["get_property", "media-title"]}' |socat - /tmp/mpvsocket
{"data":"Destiny Reacts To Boyinaband Allegations","request_id":0,"error":"success"}

➜ trakts test mpv
Testing connection with mpv.
Please ensure that mpv is playing some media.

  • Connected
  • Got info
    Playing watch at 1.82%

@iamkroot
Copy link
Owner

Are you sure you've added that title to fileinfo.title_whitelist config as mentioned in the steps above? Please check the scrobbler logs too.

@umop3plsdn
Copy link

omg i didn't see that part i am so sorry! i can't believe i missed that entirely wow. Even the install method is easier than how i was doing it locally lol. So would it be trakts config set --add fileinfo.title_whitelist "media-title"?

@iamkroot
Copy link
Owner

Yep

@umop3plsdn
Copy link

umop3plsdn commented Jan 13, 2023

Can you see if I did anything wrong?
2023-01-13-10:04:34

@umop3plsdn
Copy link

umop3plsdn commented Jan 13, 2023

I did stop/start trakts after for the record and got the same result lol. I tried off the metadata field as well since this is the output i get with socat on mpv socket: {"data":{"major_brand":"isom","minor_version":"512","compatible_brands":"isomiso2avc1mp41","title":"Vinland Saga - S01E20","encoder":"Lavf58.29.100"},"request_id":0,"error":"success"} but i got the same Playing 7c4f45. Could it still be getting it correctly just not outputting the right thing with the test flag?

@iamkroot
Copy link
Owner

You don't add the string "media-title" to whitelist. You need to add the name of the show. For the pic you shared, it would be trakts config set --add fileinfo.title_whitelist "Vinland Saga"

@umop3plsdn
Copy link

umop3plsdn commented Jan 13, 2023

thank you for clarifying! I almost thought it was that but then I second guessed and thought it was the field of choosing. Do I need to put how it shows up for media-title? for example from below "vinland-saga" all lower case or like you said "Vinland Saga"? I tested the below with "Vinland Saga". Also, I can test with another source if you think that might be the issue? Thank you for all the hard work I hope i'm not discouraging you. I'm trying to provide you with as accurate info as I can.
trakts test mpv
Testing connection with mpv.
Please ensure that mpv is playing some media.

  • Connected
  • Got info
    Playing 03f7ff340442485a8dbcbd809d2006da S85E445 at 2.15%
    [da5id@tesla] ➜ ~ echo '{"command": ["get_property", "media-title"]}' |socat - /tmp/mpvsocket| jq
    {
    "data": "vinland-saga-episode-17-dub",
    "request_id": 0,
    "error": "success"
    }

@umop3plsdn
Copy link

umop3plsdn commented Jan 13, 2023

Sweet it works on bleach! thanks for all your hard work! also i added whitelist lowercase just vinland one word and that worked too

@iamkroot
Copy link
Owner

Ah thanks for catching that. Maybe we should do case-insensitive comparison for matching titles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants