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

vf_vapoursynth: upgrade to VapourSynth API v4 #14326

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

CrendKing
Copy link
Contributor

@CrendKing CrendKing commented Jun 8, 2024

VapourSynth introduced their version 4 API in R55, 3 years ago. mpv is still using the deprecated version 3. I think it is good to migrate before VapourSynth completely removes it.

The gist of the change is documented in https://github.com/vapoursynth/vapoursynth/blob/master/APIV4%20changes.txt. The most impacted area is the video format. Previously we have a fixed table of supported formats, each represented by an integer. In v4, the integer is replaced by pointer to the full struct of the format. More details below.

Second, the way to create video filter is changed. Previously caller needs to supply a "initialization" callback to createFilter(), which sets up video info etc. In v4, video info is prepared first, passed to the createVideoFilter2() and we get back the node.

Also, previously mpv was using the fmSerial filter mode, which means VapourSynth 1) can only request one frame from mpv at a time, and 2) the order of frames requested must be sequential. I propose to change it to the parallel mode, which requests multiple frames at a time in semi random order. The reasons are, for 1), the get frame function, infiltGetFrame(), unlocks the mutex during output image generation, thus can benefit from parallel get. For 2) thanks to the frame buffer, unordered frame requests are acceptable. Just make sure the buffer is large enough.

Third, the way VapourSynth scripting environment works change. In v4, the scripting API is operated through struct pointer, much like the exist vsapi pointer. The "finalize" function is also no longer needed.

More on video formats

I took liberty to change how we handle input and output formats. Previously,

  • We use mp_autoconvert to convert input format into one of the supported VapourSynth formats, defined in the mpvs_fmt_table table.
  • The format of VapourSynth script output frame must also be from that table. We convert the frame into a mp_image for downstream.

What I did are:

  • No longer use the mpvs_fmt_table table to determine if a format is valid. Instead rely on whether a format can be handled both by mpv and VapourSynth.
  • Still restricts to only YUV formats. YUV formats are tend to be the most structured and consistent color family. Also the most popular.
  • Thanks to this, mpv now support a few new formats that were previously "disabled".
  • Due the API change, and the lack of access to VapourSynth when autoconvert is set up, the formats we sent is a superset of what's actually needed. I don't believe they do any harm, and all of them are extremely rare.

List of currently supported formats for VapourSynth

=== The following are also supported by VapourSynth resize filter ===

YUV410P8
YUV411P8
YUV440P8
YUV420P8
YUV422P8
YUV444P8

YUV420P9
YUV422P9
YUV444P9

YUV420P10
YUV422P10
YUV444P10

YUV420P16
YUV422P16
YUV444P16

=== Newly supported ===

YUV420P12
YUV422P12
YUV444P12

YUV420P14
YUV422P14
YUV444P14

YUV444PS / YUV444PF

=== The following are not supported by VapourSynth resize filter. Requires zimg when building mpv ===

YUV410PF
YUV411PF
YUV420PF
YUV422PF
YUV440PF

YUV440P10
YUV440P12

False positive input formats (all deprecated in AVPixelFormat), indistinguishable from their non-jpeg counterpart due to difference only in color range

YUVJ411P
YUVJ422P
YUVJ440P

@CrendKing CrendKing force-pushed the master branch 3 times, most recently from d7efa86 to ab99b86 Compare June 8, 2024 12:23
@kasper93
Copy link
Contributor

kasper93 commented Jun 8, 2024

Don't forget to update version check in meson.build

Copy link

github-actions bot commented Jun 8, 2024

Download the artifacts for this pull request:

Windows
macOS

@CrendKing
Copy link
Contributor Author

Don't forget to update version check in meson.build

I will. Thanks for reminding me.

@ruihe774
Copy link
Contributor

ruihe774 commented Jun 8, 2024

It is a breaking change to existing scripts. I suggest mentioning it in docs.

@CrendKing
Copy link
Contributor Author

It is a breaking change to existing scripts. I suggest mentioning it in docs.

Of course. Let me lay down what I think are user impacting:

  • Minimum VapourSynth version bump. Note that since this is only a build-time dependency change, only users who build mpv themselves with old VapourSynth are impacted. Well know builds such as shinchiro already use latest versions.
  • Additional input and output formats supported for VapourSynth scripts, namely the 3 P12 formats, 3 P14 formats and YUV444PS.
  • Configs with extremely low buffer size (e.g. buffered-frames=1:concurrent-frames=1) may need to increase a bit to accommodate the change of filter mode.

Let me know if I missed any.

@hooke007
Copy link
Contributor

hooke007 commented Jun 9, 2024

I am too busy to test. I just wonder did it improve the terrible seeking performance especially when you use the heavy vs filter?
(Compared with mpc+vs filter, mpv may spend serval seconds to recover from "pause" after seeking.)

@na-na-hi
Copy link
Contributor

na-na-hi commented Jun 9, 2024

Let me know if I missed any.

It's already mentioned. vpy script API is incompatible and some deprecated functions are removed.

@CrendKing
Copy link
Contributor Author

I am too busy to test. I just wonder did it improve the terrible seeking performance especially when you use the heavy vs filter? (Compared with mpc+vs filter, mpv may spend serval seconds to recover from "pause" after seeking.)

Well, it heavily depends on the logic of the script and plugin it uses. But I think it must came from the fmSerial filter mode mpv currently use. I bumped it to fmParallelRequests. Should improve some performance.

@CrendKing
Copy link
Contributor Author

It's already mentioned. vpy script API is incompatible and some deprecated functions are removed.

I understand. You mean the incompatibility from VapourSynth itself, as they mentioned in http://www.vapoursynth.com/2021/09/r55-audio-support-and-improved-performance/. Maybe I could just link the URL in our doc?

@ruihe774
Copy link
Contributor

ruihe774 commented Jun 9, 2024

Maybe I could just link the URL in our doc?

Sounds good.

@CrendKing
Copy link
Contributor Author

I'm trying to test the PR with build from the Actions. As kasper93 mentioned, only the MSYS2 build enables VapourSynth. Unfortunately, the MSYS2 build doesn't upload artifact. Is there particular reason why it's disabled, other than of course the Actions storage limitation? I do hope the MSYS2 artifacts are available.

@kasper93
Copy link
Contributor

Is there particular reason why it's disabled, other than of course the Actions storage limitation? I do hope the MSYS2 artifacts are available.

MSYS2 built binaries require sysroot to work, unless linked fully statically. We don't want to pack it. Those jobs are only build test, not meant to be shipped. We have MSVC and mingw cross build already.

@CrendKing
Copy link
Contributor Author

Is there particular reason why it's disabled, other than of course the Actions storage limitation? I do hope the MSYS2 artifacts are available.

MSYS2 built binaries require sysroot to work, unless linked fully statically. We don't want to pack it. Those jobs are only build test, not meant to be shipped. We have MSVC and mingw cross build already.

OK. Thanks for info.

@CrendKing CrendKing force-pushed the master branch 2 times, most recently from 6fde816 to 2cbe150 Compare June 13, 2024 12:18
Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

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

Looks good, I've not tested it excessively. But should be fine.

VapourSynth introduced their version 4 API in R55, 3 years ago. mpv is
still using the deprecated version 3. I think it is good to migrate
before VapourSynth completely removes it.

The most impacted area is the video format. Previously we have a fixed
table of supported formats, each represented by an integer. In v4, the
integer is replaced by pointer to the full struct of the format.

Second, the way to create video filter is changed. Previously caller
needs to supply a "initialization" callback to `createFilter()`, which
sets up video info etc. In v4, video info is prepared first, passed to
the `createVideoFilter2()` and we get back the node.

Also, previously mpv was using the `fmSerial` filter mode, which means
VapourSynth 1) can only request one frame from mpv at a time, and 2) the
order of frames requested must be sequential. I propose to change it to
the parallel request mode, which requests multiple frames at a time in
semi random order. The reasons are, for 1), the get frame function,
`infiltGetFrame()`, unlocks the mutex during output image generation,
thus can benefit from parallel requests. For 2) thanks to the frame
buffer, unordered frame requests are acceptable. Just make sure the
buffer is large enough.

Third, the way VapourSynth scripting environment works change. In v4,
the scripting API is operated through struct pointer, much like the
exist `vsapi` pointer. The "finalize" function is also no longer needed.
@CrendKing
Copy link
Contributor Author

CrendKing commented Jun 13, 2024

Looks good, I've not tested it excessively. But should be fine.

Thank you! I wish one more people can test before we commit, especially on the filter mode change (serial -> parallel). I probably don't use enough variety of VS plugins to cover all cases.

@kasper93
Copy link
Contributor

Thank you! I wish one more people can test before we commit, especially on the filter mode change (serial -> parallel). I probably don't use enough variety of VS plugins to cover all cases.

Real testing will be after we merge, nobody really goes into PRs and test them. Especially for more niche features.

@CrendKing
Copy link
Contributor Author

Real testing will be after we merge, nobody really goes into PRs and test them. Especially for more niche features.

Gotcha. Let's merge then. I'll try to monitor Issues see anyone report related. If I miss any, feel free to assign me.

@kasper93 kasper93 merged commit 6031a0e into mpv-player:master Jun 13, 2024
21 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.

None yet

5 participants