-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Conversation
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
Download the artifacts for this pull request: |
@@ -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) |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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
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