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

Linear light scalers can cause color shift on the output clip #90

Closed
shssoichiro opened this issue May 26, 2024 · 13 comments · Fixed by Jaded-Encoding-Thaumaturgy/vs-kernels#24
Labels
bug Something isn't working

Comments

@shssoichiro
Copy link
Contributor

Steps to reproduce:

  • Initialize a clip with the following properties:
clip = vstools.initialize_clip(
    clip,
    matrix=vstools.Matrix.SMPTE170M,
    primaries=vstools.Primaries.ST170M,
    transfer=vstools.Transfer.BT709,
)
  • Rescale the clip using the SSIM scaler:
clip = vsscale.SSIM().scale(clip, 848, 480)

In the clip that is returned, the Matrix will have been changed to BT.709, resulting in incorrect colors in the output. This occurred with a few combinations of colorimetry settings, though not all combinations.

I tested MergedFSRCNNX and noted that the issue is not present with that scaler.

@shssoichiro
Copy link
Contributor Author

It seems this is more complicated than I originally thought.

If I resize to a size that is much smaller than the original, then even if I attempt to manually fix the matrix with e.g.:

matrix = vstools.Matrix.from_video(clip)
clip = vsscale.SSIM().scale(clip, 848, 480)
clip = clip.std.SetFrameProp(prop="_Matrix", intval=matrix)

The color shifting persists.

However, if I resize to a resolution that is normally in BT.709, e.g.:

clip = vsscale.SSIM().scale(clip, 1280, 720)

(assuming the original is 1920x1080) Then the matrix changes to BT.709, but there is no color shift, i.e. it is being fully converted to the new matrix properly. So this may be some sort of conversion issue when resizing below 1024x576.

@LightArrowsEXE LightArrowsEXE added the bug Something isn't working label May 26, 2024
@shssoichiro shssoichiro changed the title SSIM scaler can incorrectly change the Matrix property on the output clip SSIM scaler can cause color shift on the output clip May 26, 2024
@Setsugennoao
Copy link
Member

Can you print the props after initialize clip? By default that will not override existing props unless you force it, so maybe the clip is marked bt709 in the metadata.

@shssoichiro
Copy link
Contributor Author

I believe I've traced down the issue, and confirmed no color shift compared to Bicubic or Merged FSRCNNX.

See Jaded-Encoding-Thaumaturgy/vs-kernels#24

@Setsugennoao
Copy link
Member

Oh lol OFC, you're not specifying the curve

@Setsugennoao
Copy link
Member

Just noticed it, i'm dumb

@shssoichiro
Copy link
Contributor Author

shssoichiro commented May 26, 2024

Should I be? I've been assuming it's optional. Though if that's the case the change I posted might need an update to work correctly if the curve is set.

Edit: Actually it looks like _curve there is always set from Transfer.from_video(self.clip)

@Setsugennoao
Copy link
Member

Yeah the function assumes you're specifying it.
If we make this optional we gotta make it None and then retrieve it from the video without heuristic
I'd rather it fail if it's not specified in curve and props

@shssoichiro
Copy link
Contributor Author

I'm a bit lost on where should be specifying curve then. Right now LinearLightProcessing seems to assume the curve based on the Transfer of the clip, and then the matrix is assumed from that, which may be incorrect. It doesn't look like we can pass a curve param into the SSIM scaler (it throws an error), and if we could it doesn't look like it would do anything with it.

@Setsugennoao
Copy link
Member

You totally can
SSIM(curve=Transfer.xxx)
Or
SSIM.scale(clip, xx, xx, curve=Transfer.xxx)

@shssoichiro
Copy link
Contributor Author

Both of those result in the following traceback (all plugins are on latest git version):

  File "/usr/lib/python3.12/site-packages/vsengine/vpy.py", line 219, in _execute
    runpy.run_path(str(script), module.__dict__, module.__name__)
  File "<frozen runpy>", line 286, in run_path
  File "<frozen runpy>", line 98, in _run_module_code
  File "/usr/lib/python3.12/site-packages/vspreview/core/vsenv.py", line 36, in _monkey_runpy_func
    glob_dict = orig_runpy_run_code(*args, **kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen runpy>", line 88, in _run_code
  File "/home/soichiro/Downloads/adv10bit.vpy", line 25, in <module>
    clip = vsscale.SSIM(curve=vstools.Transfer.BT709).scale(clip, 848, 480)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 180, in _wrapper
    return self.function(obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 314, in __call__
    return self.__get__(None, self)(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 309, in _wrapper
    return this.function(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vskernels/kernels/complex.py", line 102, in func
    ll.linear = operation(ll.linear, width, height, shift, **kwargs)  # type: ignore
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vsscale/scale.py", line 127, in _linear_scale
    l1 = self.scaler.scale(clip, width, height, shift, **(kwargs | self.kwargs))
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 180, in _wrapper
    return self.function(obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 314, in __call__
    return self.__get__(None, self)(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 309, in _wrapper
    return this.function(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vskernels/kernels/complex.py", line 249, in scale
    return super().scale(
           ^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 180, in _wrapper
    return self.function(obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 314, in __call__
    return self.__get__(None, self)(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 309, in _wrapper
    return this.function(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vskernels/kernels/complex.py", line 97, in func
    return operation(clip, width, height, shift, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 180, in _wrapper
    return self.function(obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 314, in __call__
    return self.__get__(None, self)(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 309, in _wrapper
    return this.function(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vskernels/kernels/complex.py", line 228, in scale
    clip = Scaler.scale(self, clip, width, height, shift, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 180, in _wrapper
    return self.function(obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 314, in __call__
    return self.__get__(None, self)(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/stgpytools/types/utils.py", line 309, in _wrapper
    return this.function(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vskernels/kernels/abstract.py", line 230, in scale
    return self.scale_function(clip, **_norm_props_enums(self.get_scale_args(clip, shift, width, height, **kwargs)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/vstools/utils/vs_proxy.py", line 210, in __call__
    return proxy_utils.get_vs_function(self)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "src/cython/vapoursynth.pyx", line 3069, in vapoursynth.Function.__call__
vapoursynth.Error: Bicubic: Function does not take argument(s) named curve

@shssoichiro
Copy link
Contributor Author

shssoichiro commented May 26, 2024

Huh... I wonder if this is a Vapoursynth R68 error? I don't have an instance with R66 to test against at the moment.

@Setsugennoao
Copy link
Member

Huh no you're right, I forgot i updated it
Will have to check it out tomorrow

@shssoichiro
Copy link
Contributor Author

shssoichiro commented Jun 3, 2024

Have you had an opportunity to investigate this?

As an aside, I think a fix that automates detection of the curve correctly would be better than requiring the user to specify, unless there's a solid reason to require user intervention here. This seems like it should be able to be automated e.g. this.

@shssoichiro shssoichiro changed the title SSIM scaler can cause color shift on the output clip Linear light scalers can cause color shift on the output clip Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@shssoichiro @Setsugennoao @LightArrowsEXE and others