jonniek / mpv-playlistmanager

Mpv lua script to create and manage playlists
The Unlicense
537 stars 42 forks source link

Resolve local titles #105

Closed 2084x closed 1 year ago

2084x commented 1 year ago

Looks neater now. I also clarified usage of the conf options.

The only problem I'm encountering is mpv stuttering ever so briefly when 10+ local files are loaded at the same. Very minor issue but is there a way to slow down or segment the resolve_local_titles function?

CogentRedTester commented 1 year ago

The only problem I'm encountering is mpv stuttering ever so briefly when 10+ local files are loaded at the same. Very minor issue but is there a way to slow down or segment the resolve_local_titles function?

Yes it's worth setting a limit on the number of asynchronous operations running at once. Not just for performance reasons, mpv has a limit on how many asynchronous operations can be running at once (I believe 1000 is the ceiling), so the current code will potentially crash on very long playlists. This is the case for the current url titles code as well.

jonniek commented 1 year ago

is there a way to slow down or segment the resolve_local_titles function?

My first thought is something like having a stack of files to be resolved, then in a periodic timer popping items to be resolved.

We could consider it out of scope for this PR and open another issue about it. Up to you if you want to add it to this.

I'll test this tomorrow and if it works then I'll merge(I can resolve the conflict).

2084x commented 1 year ago

We could consider it out of scope for this PR and open another issue about it.

Sounds good. I wouldn't know how to implement it properly.

jonniek commented 1 year ago

Works nicely for me! Thanks for the PR.

I'll work on the throttling today.

jonniek commented 1 year ago

I added throttling to master.

Let me know if it works & helps with your stutters.

2084x commented 1 year ago

Awesome! It helps a little bit... definitely worth having an option for it.

I'm testing loading a playlist of ~25 items, all with titles in the metadata:

=1 gives no stutters but slow =2-7 gives small audio clicks every time the stack goes through =8+ gives audio and video stutters every time the stack goes through but set it =20+ and it gets done quick with just one big stutter

not sure which one I'd prefer... :rofl:

If I wanted to eliminate stutters completely I'd probably just need better hardware right?

jonniek commented 1 year ago

Currently it's slow to resolve with 1 stack size because it only runs the timer every second. If it either looked more often or looked immediately after finishing previous resolve then it would be faster to resolve and I assume with no stutters.

Can you try with something like 0.1 or 0.05 second intervals in the title_fetch_timer with an ongoing_request_count of 1.

Is the stutter also a problem with url title resolving for you?

2084x commented 1 year ago

Can you try with something like 0.1 or 0.05 second intervals in the title_fetch_timer with an ongoing_request_count of 1.

When concurrent_title_resolve_limit=1, If I lower title_fetch_timer, resolving is very fast and without stutters. If I change ongoing_request_count=1 titles don't resolve at all(?)

The problem with using concurrent_title_resolve_limit=1 is that url resolving becomes incredibly slow even if title_fetch_timer and ongoing_request_count are changed.

Using ongoing_request_count=1 and title_fetch_timer=0.05 with concurrent_title_resolve_limit=10 suprisingly reduces stuttering and obviously resolves titles faster.

Is the stutter also a problem with url title resolving for you?

Stutters start around concurrent_title_resolve_limit=15 when resolving urls. Doesn't seem necessary to set the number that high anyway. No issues with =10 or below.

Using ongoing_request_count=1 and a lower title_fetch_timer=0.05 improves url resolving speed with large playlists as well.

Also I think --no-config should be added to the ytdl args. Saves it downloading extra data like sponsorblock segments if they are set as default in the config.

jonniek commented 1 year ago

Thanks for testing. Sorry I made a mistake, ongoing_request_count is not supposed to be modified, that's why you see weird behavior when changing it. I meant concurrent_title_resolve_limit.

title_fetch_timer=0.05 with concurrent_title_resolve_limit=10 suprisingly reduces stuttering

Quite surprising indeed 🤔

I made a change to separate ffprobe and url batching logic. I left url ones configurable with timeout and batch size. I hardcoded the title_fetch_timer to a small value 0.1, since the callback should be cheap. If the stack is empty I stop the timer until new items are pushed.

I changed Ffprobe to fetch 1 by 1 where next one is fetched immediately after finishing the previous one. This will probably be relatively fast and unnoticeable if there is no stutters. For my dir of 160 files it probes them all in 8 seconds with no frames dropped.

If it still has stutters that might be helped by delaying the next resolve by some milliseconds. You could test it by changing line 1271 to:

mp.add_timeout(0.05, local_fetching_throttler)
2084x commented 1 year ago

I made a change to separate ffprobe and url batching logic.

This was definitely the right way to go. There's no stutters or frame drops now and both local and url resolving is super quick.

eg. 300 local audio files in 13 seconds and url playlist of 200 files in 20 secs with concurrent_title_resolve_limit=10.

Great work! :+1: