mpv icon indicating copy to clipboard operation
mpv copied to clipboard

mac/vulkan: --dither-depth=auto should select 10 bit

Open m154k1 opened this issue 1 year ago • 15 comments

Important Information

Provide following Information:

  • mpv version
mpv v0.37.0-666-g77e1cfb85f-dirty Copyright © 2000-2024 mpv/MPlayer/mplayer2 projects
 built on Mar 24 2024 18:16:31
libplacebo version: v7.349.0 (v6.338.0-120-g7b294350)
FFmpeg version: git-2024-03-24-ac21582
FFmpeg library versions:
   libavutil       59.6.100
   libavcodec      61.2.100
   libavformat     61.0.100
   libswscale      8.0.100
   libavfilter     10.0.100
   libswresample   5.0.100
  • macOS Version: 14.4
  • Source of the mpv binary or bundle: git master
  • If known which version of mpv introduced the problem: https://github.com/mpv-player/mpv/commit/f914947dda421aa5903084cf1813412c8bacb003
  • Possible screenshot or video of visual glitches: N/A

Reproduction steps

Run mpv with --dither-depth=auto with any SDR video.

Expected behavior

mpv should select 10 bit dither depth.

Actual behavior

mpv selects 8 bit dither depth.

Log file

N/A


Since #13643 mpv selects 8 bit dither depth for SDR content. It might help some gpu contexts, but it's wrong for mac. macOS does internal dithering so the old behavior was correct. --dither-depth=10 does not produce banding, tested with 8 bit external display on https://github.com/mpv-player/mpv/issues/11862#issuecomment-1618340272.

The possible solution is implemented auto dither depth for macvk, similar to #13646.

m154k1 avatar Mar 24 '24 17:03 m154k1

i am wondering, back when i looked into possible solutions for opengl i made a proof of concept where i checked if one of the displays is actually driven in 10bit. back then the "System Information" also showed whether a display is in 10bit or 8bit. neither the check works anymore nor does "System Information" show any info anymore (maybe because i am on an ARM mac now). does macOS do anything in 10bit internally anyway now and dithers down if needed?

anyway some things that need to be cleared up:

  • do we need to detect display bit depth or do we always assume 10bit?
  • if we need to detect bit depth, how do we dynamically switch when the display changes?
  • when did the behaviour change that display bit depth can't be retrieved any more, and do we need to do something for backwards compatibility?

Akemi avatar Mar 24 '24 20:03 Akemi

does macOS do anything in 10bit internally anyway now and dithers down if needed?

Yes, this is how it works now.

do we need to detect display bit depth or do we always assume 10bit?

If I understand discussion in #11862 correctly, before https://github.com/mpv-player/mpv/commit/f914947dda421aa5903084cf1813412c8bacb003 the vulkan driver was responsible for selecting color depth, so maybe the best solution would be just let the driver decide again? Looks like MoltenVK does it right.

m154k1 avatar Mar 24 '24 21:03 m154k1

with the removal of this, i don't think this is possible to do in the vulkan platform context itself, since we can't just change the swapchain functions or add new ones to the static const. one probably has to readd this and extend ra_vk_ctx_params for platofrm specific functionality, though that will kill the current logic on other platforms again.

Akemi avatar Mar 24 '24 22:03 Akemi

This is by design, as explicitly done by https://github.com/mpv-player/mpv/commit/f914947dda421aa5903084cf1813412c8bacb003 commit. You don't need more than 8-bits for SDR content and if you do set dither-depth in your config. We could even select 8-bit back buffer and the result for SDR would be the same. You don't need more.

kasper93 avatar Mar 24 '24 22:03 kasper93

Vulkan has no extension to check for the current displays bit depth. So no way to automate this step. As such it is up to the user to manually set the correct dither-depth.

lextra2 avatar Mar 25 '24 05:03 lextra2

macOS does internal dithering so the old behavior was correct.

There's no real harm to dithering down to 8 bpc in mpv itself if the content isn't wide-gamut. And if the content is wide gamut, then those changes don't do anything.

However, if there's no banding on Mac at dither-depth=no (which is what happens on vulkan by default due to it picking a 16 bit backbuffer), then we could probably remove this default for Mac. Someone would have to check it first though on a Mac old enough to have 8 bit display.

Use this sample on Mac: https://github.com/mpv-player/mpv/issues/11862#issuecomment-1618340272

Bind cycle-values dither-depth "8" "no" to some key and see if there's any banding with no dithering

llyyr avatar Mar 25 '24 06:03 llyyr

Vulkan has no extension to check for the current displays bit depth. So no way to automate this step. As such it is up to the user to manually set the correct dither-depth.

what vulkan is capable is irrelevant in this case, if the specific platform has a way to check. like with d3d11.

tbh we shouldn't make just assumptions for anything. though in general less dithering is the best dithering. if we end up with 10/8bit (content) > 8 bit (mpv) > 10bit (compositor/whatever) > 10/8bit (display), i would much prefer 10/8bit (content) > 10 bit (mpv) > 10bit (compositor/whatever) > 10/8bit (display).

Akemi avatar Mar 25 '24 07:03 Akemi

tbh we shouldn't make just assumptions for anything. though in general less dithering is the best dithering

Well I had voted for dither-depth=no as the new default. This is basically the deband discussion all over again. Just let the user handle it.

lextra2 avatar Mar 25 '24 07:03 lextra2

Just let the user handle it.

They can handle it and it is recommended by the docs. But as we can see =auto mode is expected by the users to work, so we cannot remove that.

tbh we shouldn't make just assumptions for anything.

I agree that assuming target dithering based on source content is not best idea, but in the same time assumption is that this covers most of the common case without sacrificing the actual quality much and still providing the auto mode.

I think the best is to have platform specific check and/or logic to infer target depth, on Windows we have it for d3d11, could be extended for other apis with little bit of glue code probably. Little bit less convenient to get DXGI adapter in Vulkan mode though.

On macOS, I have no idea what's possible and on Linux it probably is not really possible to do anything portable.

though in general less dithering is the best dithering

Too little dithering is worse than too much dithering.

kasper93 avatar Mar 25 '24 08:03 kasper93

Using dither-depth=no on macOS is the best way probably. @Akemi what do you think about setting no for dither-depth=auto on mac?

It'd be great if somebody can test this on Intel mac / old macOS versions.

m154k1 avatar Mar 25 '24 11:03 m154k1

I think the best is to have platform specific check and/or logic to infer target depth

agreed

Too little dithering is worse than too much dithering.

i didn't say no dithering or too little, though i thought that was obvious from the context, or said differently as little dithering as possible, as much as necessary. that holds true for any kind of filtering.

Akemi avatar Mar 25 '24 17:03 Akemi

i am wondering how to proceed here?

to sum it up:

  • macOS seems to always operate at at least 10 bit internally (compositor), whether the display is 8bit or 10bit
  • previously macOS reported Framebuffer Depth: 30-Bit Color (ARGB2101010) (10bit), Framebuffer Depth: 24-Bit Colour (ARGB8888) (8bit)
  • dither-depth=no is the same as dither-depth=16?

my findings:

  • testing on a 10bit display, i can notice banding on 8bit and 10bit sources with dither-depth=8. none with dither-depth=no, dither-depth=16 or dither-depth=10
  • testing on a 8bit display (though quite an old one), i can notice a similar behaviour, though a lot less noticeable. though it could be the display
  • dither-depth should be set to at least to 10 on macOS

this leaves us with two options:

  1. we leave the selection of dither-depth to the user, keep the current behaviour of auto that it selects 8 in the SDR case and possible suboptimal quality, and close this issue
  2. change the behaviour on macOS only to select either no or 10 in the case of dither-depth=auto

for 2. i am not sure where we want to change the code to accomplish that?

Akemi avatar Oct 14 '24 13:10 Akemi

dither-depth=no as the new default, so I can remove it from my config. Just leave it to the user to cinfigure.

lextra2 avatar Oct 15 '24 03:10 lextra2

mac setting its own value for auto is probably the best way to go here. There's already an existing color_depth in ra_swapchain_fns. You can probably use that.

Dudemanguy avatar Oct 15 '24 04:10 Dudemanguy

opened a PR for this #15125.

Akemi avatar Oct 18 '24 18:10 Akemi