-
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
Python Scripting Support [RFC] #11574
base: master
Are you sure you want to change the base?
Conversation
i have some concerns regarding add python Scripting Support just like JavaScript look like i need to do more research |
Would the per interpreter-GIL in the upcoming python version allow for the elimination of a lot of the python-specific logic that's in this current implementation and let it behave more like lua/js do? |
@Dudemanguy I hope so. |
To an end user It shouldn't seem anything different to any other scripts (js/lua) although the underlying implementation is different. |
@Dudemanguy , FYI, this version of it will work with older python nicely maybe down to python3.3. |
Well if python 3.12 makes it possible to solve some of the technical challenges with embedding python in mpv, I'd think it'd be better to make that the minimum version. That's not to say that the current implementation is bad, but it'd definitely be better if python could just be yet another scripting backend. Plus it's only about a month until python 3.12 releases. |
Python 3.12 is 6 months away. I don't think that waiting for Python 3.12 should block this PR. |
Oh my mistake. I thought it was next month for some reason. |
There are some existing issues with lower level threads (threads created from the scope outside of python). On such a thread python loses its I/O wrappers (and god knows what else). These problems will still exist in 3.12. But the isolation proposed in pep-0684 can be done from the python context. In my opinion and in performance-wise, there'd be hardly any difference between the two (that is if the former approach was doable). |
This sounds very interesting! I imagine this could pave the way for a variety of scripts, including ones to easily modify the UI/UX, like context menus, and make mpv scripting more accessible to casual users |
@versedwildcat , it's still a work in progress. We have just developed a working solution to some problems that we had with embedding python on mpv. We are still a step away from getting you this feature. mpv client API hooks still need to be mapped to python functions. I am working on some part of it as I am replying to you. Thank you for your patience. |
I've converted this PR to a draft for now until it's ready for review-for-merge. |
Hello guys, Long time! |
@DeadSix27 , the issues you've mentioned should go away now. |
Github said this pr has conflicts with master branch. |
@zhongfly , There you have it. |
Same error's still.. any idea? EDIT: Sometimes I encounter this one:
I'm using your example script, it definitely has the import. |
@8lurry from mpvclient import mpv
mpv.info("Python script work!") mpv crash after script run.
gdb:
Build file can be download here: https://github.com/zhongfly/mpv-winbuild/actions/runs/5745111914 |
^ This may come if your client (testfile.py) is not valid. (i.e. testfile.py has SyntaxError or something)
^ It won't come if you have "from mpvclient import mpv" on it. |
@8lurry syntax is 100% correct and the import is also correct. EDIT: Shouldn't mpv notify if the syntax is wrong including traceback? Also same error as zhong:
|
@DeadSix27 , Could you try it again with doing the following changes?
And afterwards let me know if the issue persists. |
I have tried this,but same issue.This fix not work. |
@zhongfly , @DeadSix27 , Would you get me the complete log when you run it (please be sure to set the log level to 'debug'). |
The logs have little relevant information,I have set If I tried to run from mpvclient import mpv
mpv.info("Python script start!") # mpv crash after here
mpv.info("Python script end!") |
@zhongfly, compile it on a windows system instead. |
There is no known issue with --prefetch-playlist and file-local-options. It works just fine with them, and people have been using it since 2017 and haven't reported related issues. So ease the warnings to not scare users away from using a useful option. Also when --prefetch-playlist was implemented it also prefetched the stream cache until 559a400 removed it - see mpv-player#6753. So if such issues ever existed, they were fixed by not making it prefill the stream cache. Now it doesn't do much, it only opens a connection to the next file, and the cache of the next video starts empty.
--prefetch-playlist improves performance by starting to read the next file in the last second or playback or while viewing an image. This is obviously noticeable with playlists of direct media URLs, but can also make a difference on local slow hardware, mainly large (in bytes) images on HDDs. You can easily see the difference with 10+ MB images. Since e7a2536 and 24db17d fixed niche issues, and it's known not to cause other issues by users using since 2017 who haven't reported issues, enable it by default to improve performance. The only flaw is that it doesn't support ytdl-hook URLs, so in Youtube playlist, it does a web request with ffmpeg that is doomed to fail since the last second of playback, though this doesn't cause practical disadvantages.
The next commit will inject ASS within matching items to highlight matching positions, which means that the text key of log lines can no longer be used, and that matching items and log lines diverged enough that it's simpler to separate them. As a bonus, we can stop clearing the log and completions when searching the command history, and restore them after the search.
fzy returns which characters matched the search for every item, so use this to highlight them. This defaults to a light blue color that is readable with both black and white (selected) backgrounds. The returned positions are just bytes, so we can't put ASS tags or escape sequences around each byte because that would break displaying multibyte UTF-8 characters. Only put them around sequences of matched positions, and only when the sequences start and end at actual character positions.
This needs to strike a fine balance between being visible and having contrast with the text color and not taking attention away from the selected item. Make it slightly more opaque to differentiate it more easily. Fixes mpv-player#15711.
These are leftovers from the original repl.lua name. Also remove some redundant comments.
Because it's also called on left click.
It's most of the time redundant and there is no point to repeat it in `title`.
This allows specifying a priority list of clipboard backends to use. This is mostly useful on Linux where multiple clipboard backends exist.
Clipboard can be now disabled with --clipboard-backends-clr so --clipboard-enable is redundant.
With input.select this allows building nested submenus without the flicker of console quickly closing and reopening, and also doing multiple selections, to e.g. cycle through different values of a property, or increase a property multiple times. With input.get this is used to change the default behavior to closing by default on submit. It was argued by avih that this is more useful, and indeed Github code search shows that everybody is calling input.terminate() unconditionally on submit, so the impact of the change should be low, and restoring the old behavior is as easy as passing keep_open = true, which is just ignored in older mpv versions.
Partially revert bede446. This was removed to allow nested input.select() calls, but it is better to use the new keep_open flag for this, because it avoids the flicker of console quickly closing and reopening. Unregistering the script message allows garbage collection of variables defined in the scope that called mp.input, this is useful e.g. if you were selecting from 10k history entries.
Use the new keep_open flag to avoid flicker when opening submenus.
300 characters doesn't always fill the window. Fixes mpv-player#15944.
Be consistent with behavior of `display-stats-toggle`.
The names of single invocation key bindings for specific pages will be changed if defines user key bindings, then original invocation fails. e.g. : when user defines `key_page_4=F4`, then key binding name `stats/display-page-4-toggle` becomes `stats/display-page-F4-toggle`, and OSC menu `Help` fails.
Windows works totally differently than the other backends and is sensitive to synchronization issues since it runs its control on a separate thread. The spot where this was originally placed was a perfect storm of causing resize weirdness because it was before the flip and right after the redraw. Ideally, this is known to the windowing backend before the flip actually happens so just move this out to before the do_redraw call. It makes no practical difference for the one user of this and sidesteps the implementation differences that exist on Windows. Fixes fc9d1ca.
5370069 introduced this while implementing n-buffering. The changes are all good, but the suspect part is the implementation of wait_fence/new_fence to specifically bypass what is done generically in context.c for all other opengl backends. The commit message says that the reason for this is because "the places we create and wait for the fences needs to be somewhat different for best performance". It's certainly plausible that it may have been true then, but I see no evidence it's true now. Driver code is way different these days under the hood, and the general vo drm code has been rewritten a lot since then. The generic fencing in context.c works fine and shows no difference compared to this custom implementation. Delete it and simplify.
With the previous commits, all usage of this is gone. We can remove it for good.
Dear mpv community,
I and @8lurry are excited to introduce Python scripting support for mpv through python-embed. With this PR merged, you can use Python for scripting in mpv just how you can use Lua/JS for scripting currently. And I am sure that Python will bring numerous benefits to mpv as a scripting language like:
Some People who script mpv are using Lua to act as bridge between mpv and Python. And with Python scripting, this will stop.
We have solved Python-embed's threading issue currently thanks to @8lurry. And when we get per interpreter-GIL in Python 3.12 (https://peps.python.org/pep-0684/), it can be improved even further making the implementation analogous to Lua/Mujs implementations.
With this PR, you can run Python scripts in mpv but we still need to map many mpv functions to Python (We will be updating them in upcoming commits in PR). This PR acts as POC for time being.
We are seeking Feedback on this PR and hope to get this merged within mpv.
CC: @avih , @sfan5 , @Dudemanguy