mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.17k stars 2.89k forks source link

Screenshot color isn't consistent with displayed color #7840

Open rcombs opened 4 years ago

rcombs commented 4 years ago

Important Information

Reproduction steps

Expected behavior

Actual behavior

Log file

mpv-tagged-screenshot-output.txt

Sample files

mpv screenshot taken with screenshot-tag-colorspace=yes: tagged

mpv screenshot taken with screenshot-tag-colorspace=no: untagged

(Both taken with zimg-fast=no, though I'm not sure if it matters currently)

OS-level screenshot:

macos-screenshot

The issue is most clearly visible in the bright red areas in this screenshot (particularly the hair). Test results with these 3 images:

The tagged image can be used as a sample file to take additional screenshots of, amusingly enough; the behavior is identical to using the original source file, so I don't think there's any need for me to upload a separate sample.

I think the problem here is that screenshots are being saved with the original video gamma, and most applications expect sRGB gamma. Some (e.g. Firefox) support tagged gamma, but it's not widespread enough to rely on that (which is why screenshot-tag-colorspace was set to no by default in the first place). I've been poking around trying to get mpv to convert to sRGB gamma when taking screenshots, but I haven't had any luck so far; not entirely sure what's going wrong.

Assigning @haasn for (hopefully) some more expertise on the intricacies here.

haasn commented 4 years ago

So, there are two separate issues here:

  1. Why does mpv show what it shows when viewing those three images?
  2. Why does mpv output what it outputs when taking those three images?

Question 1 is the easier to answer. Let me first note that all three images contain the same pixel data, i.e. the only difference is down to metadata. And in terms of that, all three are different:

  1. untagged.png is indeed untagged, which the PNG specification leaves undefined but mpv defaults to assuming sRGB in this case (as it does for all untagged RGB content)
  2. tagged.png is indeed tagged, with an sRGB tag - this means that the correct interpretation is as sRGB
  3. oslevel.png contains an embedded ICC profile of some rather wide gamut color space, the copyright is set to Apple Inc., 2020. Since the space is wider than sRGB the resulting image should appear much more saturated as a result

Note that mpv ignores embedded ICC profiles unless you also have a display ICC profile set, which is probably why 3 renders the same as 1 and 2 for you - it's different on my end!

From this we can infer:

  1. Your display seems to have a non-sRGB, wide gamut color space (or at least Apple thinks so)
  2. This means you should probably be using icc-profile-auto, because all colors will be wrong out of the box otherwise - i.e. the actual image is not as saturated as it appears on your display

I assume that if you use icc-profile-auto, mpv will pick up the correct display profile and adapt the window output to it, so an OS-level screenshot of that window (with the same ICC profile embedded) should render identically to the screenshots taken with mpv.

As for why mpv creates the other two files it creates should be rather obvious from just the explanation alone that mpv considers untagged RGB to be sRGB by default, and importantly, these files should also both roundtrip back to the same thing when viewed in mpv (or other software making the same assumption).

haasn commented 4 years ago

Also, side note, it's worth exploring what happens in the current screenshot code:

  1. struct ra_fbo's color_space member gets zero-initialized (by gl_video_screenshot), which is essentially setting all fields to "auto"
  2. This gets inspected by pass_colormanage, which consults the target-trc and target-prim fields as explicit overrides and otherwise falls back to the values specified in the ra_fbo
  3. The code further down in pass_colormanage infers AUTO as being the same color space as the source, but this is never communicated back to the resulting mp_image - leaving the image_writer code to always write out sRGB tags even if the original data was not sRGB (bug!)

And for what it's worth, target-trc etc. affecting even non-window screenshots is definitely a bug, although in the past, screenshots taken via the VO were tagged by the same colorimetry data as the renderer was using - this seems to have been deleted in f17246fec and never reintroduced, the function gl_video_get_output_colorspace is currently defined and unused - another bug!

haasn commented 4 years ago

It seems to me as the solution is multitude, and involves fixing all of the following bugs:

  1. Make screenshot window tag the resulting mp_image with gl_video_get_output_colorspace again
  2. Make regular screenshot ignore target-trc/target-prim when rendering
  3. Forward the mp_image color metadata from the source to the destination when taking non-window screenshots
rcombs commented 4 years ago

Heh, yup, this is a laptop with a P3 monitor; I didn't realize mpv displayed incorrectly in that case by default (I thought macOS automatically handled color conversion in the compositor). I guess it was just expanding the BT709 input into a P3 output? Kind of a pity it was wrong, I was kinda fond of that bright shade.

icc-profile-auto does indeed make mpv's display output match its own screenshots:

image

Oddly, I tested in QuickTime and got yet another slightly different result; not sure what's going on here:

image

Wonder if it'd be worth enabling icc-profile-auto by default on macOS, then? All iMacs since 2015 and MacBook Pros since 2016 have had P3 displays with well-calibrated built-in default profiles, so they're currently oversaturating by default on BT709 content. I'd just note that the "ICC profile detected contrast very high" warning would probably need to be dealt with, as the built-in profiles specify infinite contrast; not sure how that can best be addressed.

haasn commented 4 years ago

Kind of a pity it was wrong, I was kinda fond of that bright shade.

For subjective aesthetic reasons it's definitely an effect you can mimick using a custom "gamut expansion" shader. I do agree that saturated colors make anime etc. look very nice, the main three issues afaict are skin tones, grass and sky. Maybe we can develop a shader that tries preserving those three hue types while oversaturating everything else (especially red)?

Almost like a sort of "inverse gamut mapping", i.e. mostly linear response with "un-knee" function close to the gamut boundary.

haasn commented 4 years ago

Wonder if it'd be worth enabling icc-profile-auto by default on macOS, then? All iMacs since 2015 and MacBook Pros since 2016 have had P3 displays with well-calibrated built-in default profiles, so they're currently oversaturating by default on BT709 content. I'd just note that the "ICC profile detected contrast very high" warning would probably need to be dealt with, as the built-in profiles specify infinite contrast; not sure how that can best be addressed.

I would enable it by default on all platforms...

haasn commented 4 years ago

For the "infinite contrast" warning, re-analyzing this problem I'm not sure if what we're doing currently is even correct. For "perceptual" (black-scaled) profiles, we probably don't want to also be performing contrast scaling in the BT.1886 implementation, so rather than treating it as 1000:1 contrast we should probably treat it more like infinite contrast, either at gamma 2.4 or possibly at gamma 2.2? (Or indeed, the dreaded 1.961!)

I unfortunately don't have any perceptual profiles to test with, so I can't say what preserves the overall intent the best, end-to-end. Feel free to open a new issue for that. Maybe I can get around to dusting off my colorimeter and improving the status quo of perceptual profile support.

It would be nice if apple shipped both perceptual and colorimetric profiles for their displays, or something..

SingleRottenChips commented 7 months ago

In my case (macOS 14.2.1), screenshots match mpv's display with icc-profile-auto, but only when they are untagged, i.e. without screenshot-tag-colorspace, which took me a long time to figure out.


Update: Sorry, it's got nothing to do with "tag" but with target-trc which should be set to srgb, at least for SDR content.

But I feel like the display is over-saturated in dark scenes.

icc-profile-auto off: sc_upscaled--

icc-profile-auto on: 0001

off: large_phantom_of_the_paradise_02_blu-ray_

on: Phantom of the Paradise 1974 720p BluRay (00:20:54 462) 0002