Description improvement and some behavior change for sdr-adjust-gamma #17007
Conversation
|
@kasper93 Do you think this is ok? |
DOCS/man/options.rst
Outdated
| both source and target metadata is correct. | ||
| both source and target metadata is correct. If you are using macOS, Windows | ||
| with ACM enabled, or Wayland that compositor has color management protocol | ||
| support, metadata should be always correct. |
There was a problem hiding this comment.
What does color management have to do with source metadata?
There was a problem hiding this comment.
Metadata is provided by compositor's color management API, isn't it?
There was a problem hiding this comment.
This option requires that both source and target metadata is correct. The compositor has nothing to do with the source, it doesn't even know anything about the source file.
There was a problem hiding this comment.
This option requires that both source and target metadata is correct. The compositor has nothing to do with the source, it doesn't even know anything about the source file.
Then my opinion is to change this part to “output metadata should be always correct”, is this ok?
There was a problem hiding this comment.
I prefer saying "On color managed platforms, the output metadata will most likely be correct" instead of the current way of listing gpu-apis and platforms with their settings
There was a problem hiding this comment.
I've updated description, what's your opinion?
Headcrabed
force-pushed
the
description_improvement
branch
from
6b1a431 to
bdbb1d5
Compare
|
Also |
What do you mean? icc-profile takes precedence over any other target options. |
So |
Yes. Is there evidence that would say otherwise? |
When I'm testing under windows+ACM off and set |
Headcrabed
force-pushed
the
description_improvement
branch
from
bdbb1d5 to
fa4801a
Compare
| case PL_COLOR_TRC_BT_1886: | ||
| case PL_COLOR_TRC_GAMMA22: | ||
| case PL_COLOR_TRC_SRGB: | ||
| #ifdef __APPLE__ |
There was a problem hiding this comment.
Color management is also always active on Wayland if the compositor gives us a preferred image description, as well as on Windows with ACM/HDR. This should do the same for all 3 platforms
There was a problem hiding this comment.
You are right, but I'm not quite sure how to port my ACM detection code and wayland check code to MPV... Could you help?
There was a problem hiding this comment.
Check if we receive wp_image_description_info_v1::done and that it gives us an image description
There was a problem hiding this comment.
I mean, which file should I put those new functions in, and which function should be used to call those new functions? 🤔
Headcrabed
force-pushed
the
description_improvement
branch
from
fa4801a to
6c4c803
Compare
|
Download the artifacts for this pull request: Windows |
DOCS/man/options.rst
Outdated
|
|
||
| However, your render API should also support passing output metadata to MPV. | ||
| Setting ``--gpu-api`` to ``d3d11`` or ``vulkan`` (``opengl`` won't work) | ||
| would allow this. |
There was a problem hiding this comment.
Generally, it is recommended to enable this option, if you can ensure that both source and target metadata is correct. On color managed platforms, the output metadata will most likely be correct. Such platforms include macOS, Windows with ACM and Wayland compositors with color management protocol support.
The part about render API is not necessary or even correct because Vulkan doesn't tell us anything about the target metadata. On Windows, we get it via D3D11 and on Wayland we get it via the protocol. This information is completely agnostic of the rendering API used, opengl just won't do any hinting unless we use the wayland protocol for it.
There was a problem hiding this comment.
Emmm you don't treat VK_COLOR_SPACE as metadata source?
There was a problem hiding this comment.
I changed the description here again, do you think this is ok?
It doesn't matter. named transfer function is not used when icc profile is applied or any other lut. |
Headcrabed
force-pushed
the
description_improvement
branch
from
6c4c803 to
d202cb3
Compare
Add some description about output metadata. Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
…no" on macOS On macOS, global color management is always enabled, so there's no need to make "auto" behaves like "no". Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
Headcrabed
force-pushed
the
description_improvement
branch
from
d202cb3 to
f24dec3
Compare
3 participants
No description provided.