mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
26.76k stars 2.84k forks source link

A way to pass arbitrary user data to VapourSynth script #14214

Closed CrendKing closed 1 month ago

CrendKing commented 1 month ago

Expected behavior of the wanted feature

Currently the vapoursynth video filter does not accept any argument for passing arbitrary user data. This still limits what the VS script can do.

Ideally, vapoursynth can have an user-data option that contains string value. mpv passes that value to the VS script just like container_fps and others. There could be a reasonable cap of the data size like 100. Once the VS script gets the data, it can do all sorts of data extraction and transformation.

Another benefit is that instead of mpv always have to catch up to user needs for this filter, with this users can just pass whatever needed themselves, thus future requests to add more properties can reduce.

Alternative behavior of the wanted feature

Main reason I'm asking this that the current alternatives are lacking. I've tried

  1. Install a Lua extension that can call OS API (e.g. winapi for Windows), set environment variable from Lua side, then mp.commandv('set', 'vf', 'vapoursynth=mpv.vpy'). Then in mpv.vpy, try to retrieve that data with os.environ. I noticed that even the variable is correctly present in Process Explorer, os.environ doesn't have that variable. I guess even though os.environ says it captured the mapping first time the osmodule is imported, mpv probably loads VapourSynth and thus the os module before my Lua script, so changing the process environment variables in Lua doesn't work.
  2. In Lua, write to a file (e.g. C:\test), then load file in VS script. This works, but then 1) someone has to cleanup that file; 2) what happens if multiple mpv instances with my scripts concurrently access that file? Ideally the file name should have the current process ID in it to avoid collision, but then that further complicates the task.
  3. Any other usual IPC mechanism such as mailslots require deep integration with the OS API, which Lua lacks.

Log File

No response

Sample Files

No response

CrendKing commented 1 month ago

@kasper93 Thanks to your recent contributions about building mpv on Windows (e.g. https://github.com/mpv-player/mpv/commit/112fa549beb52ab3d75999c8d8a82936e13001c4), I can now contribute myself with ease.

If interested, I created PR at https://github.com/mpv-player/mpv/pull/14312.

CrendKing commented 1 month ago

For documentation purpose, I still encounter some issues when I try to build mpv on Windows after following the up-to-date instructions, specifically the "Native Compilation with Clang (Windows SDK Build)" route:

  1. There is no mentioning of activating the Visual Studio developer environment. It is included in the CI. I know this step is obvious, but I don't think it hurts to mention.
  2. Since the build script heavily relies on the Meson subprojects, the "Update Meson WrapDB" step from CI is actually crucial. Without it, my build was originally pulling harfbuzz 5.2 from some upstream, which doesn't compile due to old issue long fixed. I think we should either mention in the instruction or update the build script.
  3. CMake is required for shaderc (which is enabled by default), but not mentioned.
  4. pkg-config might be required if certain components are enabled. For instance, VapourSynth is not in Meson WrapDB, and the easiest way to fulfill the dependency is a pkg-config file like this:
    
    prefix=D:/vapoursynth/current/sdk
    includedir=${prefix}/include/vapoursynth
    libdir=${prefix}/lib64

Name: vapoursynth Description: VapourSynth Version: 68 Libs: -L${libdir} -lVapourSynth.lib Cflags: -I${includedir}


5. [LuaJIT requires `sed` to configure](https://github.com/mesonbuild/wrapdb/blob/e9e0fa18b4c01acd8f5b0d9fd375cde9994a50c3/subprojects/packagefiles/luajit/src/meson.build#L27), which is not available on Windows by default. Since Git on Windows includes it, maybe a brief mention to change `$env:PATH` like `$env:PATH += ';<git_directory>/usr/bin'`?
kasper93 commented 1 month ago

I agree with you on those points.

  1. I didn't mention it, because I have it directly integrated in terminal, but we can mention to open Developer PowerShell for VS or something. Windows Terminal automatically adds detected VS installations to its selection.
  2. This is unfortunate, because freetype have committed wraps in the repo, which are outdated. Indeed this step should be explicitly mentioned same as in cross-compilation section https://github.com/mpv-player/mpv/blob/31ae111ff577deb46aa0fc80732fddae65a59b36/DOCS/compile-windows.md?plain=1#L47-L54
  3. Correct.
  4. and 5. We could mention about luajit, because it is enabled in the script, but for vapoursynth and all other possible dependencies, we have to stop somewhere, it is exercise for the reader to resolve remaining issues and build things they may need.

Feel free to send PR with your suggestions, it is good to have someone else to validate the workflow and identify issues like you did.

CrendKing commented 1 month ago

Feel free to send PR with your suggestions, it is good to have someone else to validate the workflow and identify issues like you did.

Here you are: https://github.com/mpv-player/mpv/pull/14315