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

Artifacts with gpu-hq profile for some videos #6111

Closed
adisib opened this issue Sep 5, 2018 · 9 comments
Closed

Artifacts with gpu-hq profile for some videos #6111

adisib opened this issue Sep 5, 2018 · 9 comments

Comments

@adisib
Copy link

adisib commented Sep 5, 2018

This can be narrowed down to just the dscale=mitchell and sigmoid-upscaling=yes settings from the gpu-hq profile.

mpv version and platform

0.29.0 2018-07-31 build by lachs0r on Windows 10 64-bit

Reproduction steps

Have --dscale=mitchell --sigmoid-upscaling=yes. This does not occur for all videos, but occurs on some.

Expected behavior

Look something like this (screenshot without sigmoid-upscaling to remove the artifacts):
https://postimg.cc/image/tetd2g24h/

Actual behavior

White pixels around black points, as can be seen in this screenshot (Print Screen key was used, because it doesn't show up when pressing 's' to get a screenshot with mpv):
https://postimg.cc/image/qkq7ozkip/

Log file

https://pastebin.com/eCDwYzWB

Sample files

An accessible example would be something like this youtube video with youtube-dl (at 1440p for me, the 1080p version does not for me), which exhibits it on the first frame: https://www.youtube.com/watch?v=LXb3EKWsInQ

e.g.

mpv --no-config --pause --ytdl-format="bestvideo[height<=?1440][vcodec!=?vp9.2]+bestaudio/best" --cache-initial=0 https://www.youtube.com/watch?v=LXb3EKWsInQ --dscale=mitchell --sigmoid-upscaling=yes
@haasn
Copy link
Member

haasn commented Sep 5, 2018

Can't reproduce on my end, but seems like some sort of underflow bug. What's worrying me is the fact that --sigmoid-upscaling=yes should be a no-op when downscaling HDR content.

@CounterPillow
Copy link
Contributor

Can reproduce on Linux with an Intel GPU.
mpv-shot0001

@haasn
Copy link
Member

haasn commented Sep 5, 2018

This generated shader looks fishy. From your log:

[   6.641][d][vo/gpu/d3d11] [ 29] // scaler pre-conversion
[   6.641][d][vo/gpu/d3d11] [ 30] // linearize
[   6.641][d][vo/gpu/d3d11] [ 31] color.rgb = clamp(color.rgb, 0.0, 1.0);
[   6.641][d][vo/gpu/d3d11] [ 32] color.rgb = pow(color.rgb, vec3(2.4));
[   6.641][d][vo/gpu/d3d11] [ 33] color.rgb *= vec3(1.0/1.000000);
[   6.641][d][vo/gpu/d3d11] [ 34] // main scaling
[   6.641][d][vo/gpu/d3d11] [ 35] out_color = color;

Compare with the source:

    // Pre-conversion, like linear light/sigmoidization
    GLSLF("// scaler pre-conversion\n");
    bool use_linear = p->opts.linear_scaling || p->opts.sigmoid_upscaling;

    // Linear light downscaling results in nasty artifacts for HDR curves due
    // to the potentially extreme brightness differences severely compounding
    // any ringing. So just scale in gamma light instead.
    if (mp_trc_is_hdr(p->image_params.color.gamma) && downscaling)
        use_linear = false;

    if (use_linear) {
        p->use_linear = true;
        pass_linearize(p->sc, p->image_params.color.gamma);
        pass_opt_hook_point(p, "LINEAR", NULL);
    }

It seems as though mp_trc_is_hdr(p->image_params.color.gamma) is false on your end. Moreover, based on the generated linearization code (gamma 2.4) it seems as though p->image_params.color.gamma itself is equal to MP_CSP_TRC_BT_1886 on your end. This is highly unusual.

On my end, the image_params are correctly detected and the linearization code is skipped. I'll try again soon-ish with ffmpeg and mpv git master.

@dnmTX
Copy link

dnmTX commented Sep 5, 2018

I can also reproduce it.Same config as OP just had to add --vo=gpu --gpu-context=d3d11 --hwdec=d3d11va otherwise it was using software decoding. Windows 10 1607

mpv

Log File

log.txt

@Jj0YzL5nvJ
Copy link

Jj0YzL5nvJ commented Sep 5, 2018

I can reproduce on AMD GPU too.
https://0x0.st/sv_H.png
https://0x0.st/sv_-.txt

mpv --no-config --pause --ytdl-format="bestvideo[height<=?1440][vcodec!=?vp9.2]+bestaudio/best" --cache-initial=0 https://youtu.be/LXb3EKWsInQ?t=2m7s --dscale=mitchell --sigmoid-upscaling=yes --fs

@laichiaheng
Copy link

laichiaheng commented Sep 5, 2018

It has the problem with 1440p format on AMDGPU, but no problem with 2160p.
1440p: https://0x0.st/sv_B.txt
1440p

2160p: https://0x0.st/sv_M.txt
2160p

@haasn
Copy link
Member

haasn commented Sep 5, 2018

Okay, I can reproduce the issue on my end. Seems like the choice of format is relevant. Format 337 (2160p60 vp9.2) does not exhibit the issue, format 315 (2160p60 vp9) does.

From looking at the metadata of the clip it's immediately obvious what the issue is:

[mkv] Found the head...
[mkv] + a segment...
[mkv] Parsing seek head...
[mkv] |+ segment information...
[mkv] float 313780.000000
[mkv] | + muxing app: google/video-file
[mkv] | + writing app: google/video-file
[mkv] | + timecode scale: 1000000
[mkv] | + duration: 313.780s
[mkv] |+ segment tracks...
[mkv] | + a track...
[mkv] |  + Track number: 1
[mkv] |  + Track type: Video
[mkv] |  + Video track
[mkv] |   + Pixel width: 3840
[mkv] |   + Pixel height: 2160
[mkv] |    + Matrix: bt.709
[mkv] |    + Primaries: bt.709
[mkv] |    + Gamma: bt.1886
[mkv] |    + Levels: limited
[mkv] |    + MaxCLL: 1100
[mkv] |  + Codec ID: V_VP9
[mkv] |  + Default duration: 33.367ms ( = 29.970 fps)

It's marked as an SDR clip, but it still contains HDR metadata, leading to a scenario where the components in mpv disagree about whether or not the clip is HDR or not. The main scaling code, which goes by the tagged TRC, says it's SDR (which it is) - but the tone mapping code, which goes by the tagged signal peak, performs tone mapping (which given the tagged metadata it would rightly need to do so).

So this is an issue both with the source and mpv: the source should not mistag their crap, and mpv should sanitize mistagged crap to avoid blowing up on SDR clips with HDR metadata.

(Although to be fair, an SDR TRC can be used with floating point formats to produce a HDR file without requiring a HDR TRC, but I think this is such an unlikely scenario that I'm willing to lose this support in order to avoid exploding on clips like this - and besides, our code isn't exactly equipped to handle that case anyway)

haasn added a commit to haasn/mp that referenced this issue Sep 5, 2018
By overriding it with 1.0 (aka SDR). This prevents blowing up on
mistagged clips.

Fixes mpv-player#6111
sfan5 pushed a commit that referenced this issue Sep 5, 2018
By overriding it with 1.0 (aka SDR). This prevents blowing up on
mistagged clips.

Fixes #6111
@haasn
Copy link
Member

haasn commented Sep 6, 2018

It just occurred to me that maybe we should clamp it to <=1.0 rather than forcing it to 1.0, to avoid excluding the theoretical possibility of having an SDR clip with a lower signal peak. But, eh.

@dnmTX
Copy link

dnmTX commented Sep 6, 2018

There is something else i've noticed(could be a bug).So the video sample from the first post if you try to play it without --cache-initial=0 won't even start.There is a message showing that can not cache uncacheable streams or w/e,but if you look at the reference manual it says:

--cache-initial=
Playback will start when the cache has been filled up with this many kilobytes of data (default: 0)

So is it set to 0 or not? If it is the video theoretically should start playing without inserting an extra command line.

jeeb pushed a commit to jeeb/mpv that referenced this issue Sep 29, 2018
By overriding it with 1.0 (aka SDR). This prevents blowing up on
mistagged clips.

Fixes mpv-player#6111

(cherry picked from commit 48c38f7)
jeeb pushed a commit to jeeb/mpv that referenced this issue Sep 29, 2018
By overriding it with 1.0 (aka SDR). This prevents blowing up on
mistagged clips.

Fixes mpv-player#6111

(cherry picked from commit 48c38f7)
jeeb pushed a commit to jeeb/mpv that referenced this issue Sep 29, 2018
By overriding it with 1.0 (aka SDR). This prevents blowing up on
mistagged clips.

Fixes mpv-player#6111

(cherry picked from commit 48c38f7)
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

No branches or pull requests

6 participants