EleonoreMizo / fmtconv

Format conversion tools for Vapoursynth and Avisynth+
Do What The F*ck You Want To Public License
67 stars 14 forks source link

Matrix col_fam and the cm* constants in future VS versions #22

Closed myrsloik closed 2 years ago

myrsloik commented 3 years ago

I changed most of the constant values in the new version of the VS API but due to how things are structured filters that use the cm* (cmRGB, cmYUV...) constants from a different API version may leak through. Since fmtconv's matrix filter is the only external one that uses these constants it's probably easier to ask you to implement the workaround than to inspect and manipulate all the arguments whenever you filter is invoked.

Basically you need to check against both the API3 and API4 values. Here's how ShufflePlanes does it: https://github.com/vapoursynth/vapoursynth/blob/doodle1/src/core/simplefilters.cpp#L561

Note that in the new version YCoCg has been removed as a separate format and is now considered to be "YUV". The experimental new builds can be found here: https://forum.doom9.org/showthread.php?t=183070

A longer summary of the issue here: https://github.com/vapoursynth/vapoursynth/issues/726

EleonoreMizo commented 3 years ago

Thank you for letting me know the issue. I’m going to check this more deeply and find a workaround. But first I hope I could find a way to install recent VS versions, because Python 3.9 cannot be installed on Win 7.

myrsloik commented 3 years ago

I think it's easier to stay on R52 for now if you need to use Python 3.8. R53 and R54 were mostly Linux fixes anyway. If you manage to find a large number of upset users maybe I'll provide python 3.8 compatible builds. Or think of some way to support multiple python versions at once.

sl1pkn07 commented 3 years ago

https://github.com/EleonoreMizo/fmtconv/commit/eb847d926a28ee1bd1c1b089cbab5118954c12cc

this change breaks the build when use system vapoursynth instead of bundled one (cross-related https://github.com/vapoursynth/vapoursynth/issues/726)

greetings

EleonoreMizo commented 3 years ago

What do you mean? Could you give more details?

sl1pkn07 commented 3 years ago

simply remove the bundled VapourSynth.h header and run the configure with this: (of course. need install vapoursynth before (in my case linux environment))

cd "${_plug}/build/unix"
CPPFLAGS+=" $(pkg-config --cflags vapoursynth)" \
  ./configure \
    --prefix=/usr \
    --libdir=/usr/lib/vapoursynth

this add the systemwide installation of vapoursynth and add the include path to the build system

tested with R54 from master and R55 from doodle1 branch

greetings

sl1pkn07 commented 3 years ago

i think move this https://github.com/EleonoreMizo/fmtconv/commit/eb847d926a28ee1bd1c1b089cbab5118954c12cc#diff-f306909e14a5f19241eaa5c52cbf3a1937cd62f17cb869cad1f3fb1822953a8d to other file/header solve the problem

EleonoreMizo commented 3 years ago

O.K., thanks. I’ll find a way to do it cleanly.

EleonoreMizo commented 3 years ago

Fixed in 67dab4adc11b11962c4f0c18dc4b45e5e8c974c4 I now include a small wrapping file that adds the v4 constants, instead of directly including VapourSynth.h.