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

Change auto-dithering to DitherType.ORDERED and ERROR_DIFFUSION where applicable #84

Closed
wants to merge 1 commit into from

Conversation

LightArrowsEXE
Copy link
Member

Courtesy of @wiwaz. From our PMs:

@wiwaz:

i did tests
at high bitdepths the difference between ordered and error diffusion spatially is unnoticeable
but ordered is static
using error diffusion all the time is hurting our compressability
and maybe even making quality worse
since its adding random temporal noise that we really dont even need
again, the difference is unnoticeable spatially at high depths
ordered only looks ugly when dithered down to 8
so basically when we are doing 32 to 16 or 16 to 10 whatever we should really be considering ordered instead
and opting for error diffusion only for the final dither down

With this in mind, this PR is more of a call to action for additional testers to see how well this works out in real-life application. Please test! @NSQY @Setsugennoao

Courtesy of @wiwaz. From our PMs:

@wiwaz:
```
i did tests
at high bitdepths the difference between ordered and error diffusion spatially is unnoticeable
but ordered is static
using error diffusion all the time is hurting our compressability
and maybe even making quality worse
since its adding random temporal noise that we really dont even need
again, the difference is unnoticeable spatially at high depths
ordered only looks ugly when dithered down to 8
so basically when we are doing 32 to 16 or 16 to 10 whatever we should really be considering ordered instead
and opting for error diffusion only for the final dither down
```

With this in mind, this PR is more of a call to action for additional testers to see how well this works out in real-life application. Please test! @NSQY @Setsugennoao
@Setsugennoao
Copy link
Member

Should be based on input and output bitdepth, not just blindly use ordered (which i don't like tbh...)

@LightArrowsEXE
Copy link
Member Author

LightArrowsEXE commented Sep 19, 2023

Courtesy of @NSQY (PLEASE POST HERE >:((( ). It seems there's also a massive speed-gain to be had here.

>>> from vstools import core, vs, clip_async_render
>>> core.num_threads = 1
>>> clip = core.std.BlankClip(None, 3840, 2160, format=vs.YUV444PS, length=500, keep=True
... )
>>> clip_async_render(clip.resize.Point(format=vs.YUV444P16, dither_type='none'), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 257.48 fps 0:00:00
>>> clip_async_render(clip.resize.Point(format=vs.YUV444P16, dither_type='ordered'), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 259.63 fps 0:00:00
>>> clip_async_render(clip.resize.Point(format=vs.YUV444P16, dither_type='error_diffusion'), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 53.58 fps 0:00:00

Also something to look at: fmtc dithering types. This may be a good time to implement them, even if we decide not to use them by default for depth.

EDIT:

>>> clip_async_render(clip.fmtc.bitdepth(bits=16, dmode=1), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 175.46 fps 0:00:00
>>> clip_async_render(clip.fmtc.bitdepth(bits=16, dmode=0), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 175.46 fps 0:00:00
>>> clip_async_render(clip.fmtc.bitdepth(bits=16, dmode=6), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 7.84 fps 0:00:00
>>> clip_async_render(clip.fmtc.bitdepth(bits=16, dmode=9), None, 'Test')
Test ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 500/500 100.00% 70.34 fps 0:00:00

@ghost
Copy link

ghost commented Sep 19, 2023

Should be based on input and output bitdepth, not just blindly use ordered (which i don't like tbh...)

Ordered specifically is terrible for dithering down to 8 bits so yes it'd have to be dependant on in/out bits unless a more robust dithering algorithm (void?) is used.

@LightArrowsEXE
Copy link
Member Author

Sorted in #85.

@LightArrowsEXE LightArrowsEXE deleted the ordered-dithering branch May 21, 2024 05:34
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.

2 participants