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

osc.lua: observe playlist-count instead of playlist property #15265

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

llyyr
Copy link
Contributor

@llyyr llyyr commented Nov 5, 2024

Observing playlist property with "native" type is too expensive for
larger playlists, and we only observe this property to
activate/deactivate next/prev buttons on the osc. We actually only need
to know if the playlist-count has changed, nothing else.

Fixes: #15264

Observing playlist property with "native" type is too expensive for
larger playlists, and we only observe this property to
activate/deactivate next/prev buttons on the osc. We actually only need
to know if the playlist-count has changed, nothing else.

Fixes: mpv-player#15264
@llyyr llyyr changed the title osc.lua: observe track-list/playlist property with type nil osc.lua: observe playlist-count instead of playlist property Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

Download the artifacts for this pull request:

Windows
macOS

@@ -2562,7 +2562,7 @@ end
mp.register_event("shutdown", shutdown)
mp.register_event("start-file", request_init)
mp.observe_property("track-list", "native", request_init)
mp.observe_property("playlist", "native", request_init)
mp.observe_property("playlist-count", "native", request_init)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should lead by example and observe with nil if the value is not needed
(actually that seems to also apply to track-list?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were changed to "native" in de08494 to avoid encouraging people to imitate the code, but I guess you're right, both can be nil.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We changed to native in de08494

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine for me either way

@sfan5 sfan5 merged commit 168fb56 into mpv-player:master Nov 6, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading next/previous item gets progressively slower with longer playlists
3 participants