search

mpv-player / mpv

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

Polar scalers behave weird with vo=gpu on Android #13886

Open obscenelyvague opened 4 months ago

obscenelyvague commented 4 months ago

Important Information

Provide following Information:

Reproduction steps

Output with config no-config vo=gpu hwdec=no scale=lanczos lanczos

Output with config no-config vo=gpu hwdec=no scale=ewa_lanczos ewa_lanczos

FWIW, output with config no-config vo=gpu-next hwdec=no scale=ewa_lanczos ewa_lanczos_gpu-next

Expected behavior

Polar scalers upscale videos normally on vo=gpu, as they do on vo=gpu-next

Actual behavior

Visual artifacts show up when upscaling videos with polar scalers on vo=gpu

Log file

lanczos.log ewa_lanczos.log

ewa_lanczos_gpu-next.log

Sample files

https://github.com/mpv-player/mpv/assets/36733368/e72916c0-8838-4330-a89d-781276de300a

sfan5 commented 4 months ago

95% chance that this is a bug in the crappy vendor driver.

kasper93 commented 4 months ago

Are you able to test with this patch?

diff --git a/video/out/gpu/video_shaders.c b/video/out/gpu/video_shaders.c
index e202818501..3da9456a75 100644
--- a/video/out/gpu/video_shaders.c
+++ b/video/out/gpu/video_shaders.c
@@ -171,6 +171,7 @@ void pass_sample_polar(struct gl_shader_cache *sc, struct scaler *scaler,

             if (!sup_gather)
                 use_gather = false;
+            use_gather = false;

             if (use_gather) {
                 // Gather the four surrounding texels simultaneously

I don't have time to look into details right now, but I suspect there might be issue when switching between gather and direct sampling, where we sample already gathered values twice.

obscenelyvague commented 4 months ago

Hi @kasper93, last time there was some cryptic error, maybe I can try building from source again later. For the time being, using shaders, I found that gather version of this shader exhibits the same issue while the normal version does not.

kasper93 commented 4 months ago

I see vo_gpu is pretty ignorant when checking for gather support.

https://github.com/mpv-player/mpv/blob/ab504514b874c66fa83dd17cafd4092220138a2b/video/out/opengl/ra_gl.c#L122-L123

While vo_gpu_next check the actual gather support.

if (gl_test_ext(gpu, "GL_ARB_texture_gather", 40, 0)) {
    get(GL_MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB, &p->gather_comps);
    get(GL_MIN_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &glsl->min_gather_offset);
    get(GL_MAX_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &glsl->max_gather_offset);
}
use_gather &= PL_MAX(x, y) <= sh_glsl(sh).max_gather_offset;
use_gather &= PL_MIN(x, y) >= sh_glsl(sh).min_gather_offset;

From your log it looks like driver correctly says that gather is not supported, but vo_gpu still tries.

kasper93 commented 4 months ago

Something like the following will be enough to patch vo_gpu. Not fully correct, because it should check if actual offset is enoguh, but I don't care about vo_gpu enoguh to spend much time on it. Patches welcome.

diff --git a/video/out/opengl/ra_gl.c b/video/out/opengl/ra_gl.c
index f535f1f13d..402561a7d1 100644
--- a/video/out/opengl/ra_gl.c
+++ b/video/out/opengl/ra_gl.c
@@ -119,8 +119,16 @@ static int ra_init_gl(struct ra *ra, GL *gl)
     }

     // textureGather is only supported in GLSL 400+ / ES 310+
-    if (ra->glsl_version >= (ra->glsl_es ? 310 : 400))
-        ra->caps |= RA_CAP_GATHER;
+    if (ra->glsl_version >= (ra->glsl_es ? 310 : 400)) {
+        GLint gather_comps = 0;
+        GLint min_gather_offset = 0;
+        GLint max_gather_offset = 0;
+        gl->GetIntegerv(GL_MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB, &gather_comps);
+        gl->GetIntegerv(GL_MIN_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &min_gather_offset);
+        gl->GetIntegerv(GL_MAX_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &max_gather_offset);
+        if (gather_comps && min_gather_offset && max_gather_offset)
+            ra->caps |= RA_CAP_GATHER;
+    }

     if (gl->BlitFramebuffer)
         ra->caps |= RA_CAP_BLIT;
sfan5 commented 4 months ago

I see vo_gpu is pretty ignorant when checking for gather support.

Well, not really. Gather became part of GL 4.0 and GLES 3.1 so the version check is correct. It indeed doesn't check what the driver says about min/max offset, but I also didn't see what the standard says about that (required minimum?).

kasper93 commented 4 months ago

In fact libplacebo fails to get gpu caps. Probably should be fixed on libplacebo side.

[2599.797][v][vo/gpu-next/libplacebo] GPU does not seem to support lossless texture readback, restricting readback capabilities! This is a GLES/driver limitation, there is little we can do to work around it.
[2599.797][e][vo/gpu-next/libplacebo] pl_gpu_create_gl: OpenGL error: GL_INVALID_OPERATION
[2599.797][w][vo/gpu-next/libplacebo] Encountered errors while detecting GPU capabilities... ignoring, but expect limitations/issues
[2599.797][v][vo/gpu-next/libplacebo] GPU information:
[2599.797][v][vo/gpu-next/libplacebo]     GLSL version: 300 es
[2599.797][v][vo/gpu-next/libplacebo]       subgroup_size:             0
[2599.797][v][vo/gpu-next/libplacebo]       min_gather_offset:         0
[2599.797][v][vo/gpu-next/libplacebo]       max_gather_offset:         0

Looking at some devices with Adreno 650 at https://opengles.gpuinfo.org/ gather should be supported. Now the question is, if it is really not working or there is some bug in mpv, but I suspect former.

This needs some debugging to understand what is exactly the correct behavior.

EDIT:

@obscenelyvague: Could you run https://play.google.com/store/apps/details?id=de.saschawillems.glescapsviewer on your device, just to be sure we look at correct caps.

@haasn: Is this expected that libplacebo fails to get gpu caps? While other software it working?

obscenelyvague commented 4 months ago

Got it to build. I can confirm that explicitly disabling gather with first patch works. Second patch fails with this error though:

../video/out/opengl/ra_gl.c:126:25: error: use of undeclared identifier 'GL_MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB'
        gl->GetIntegerv(GL_MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB, &gather_comps);
                        ^
../video/out/opengl/ra_gl.c:127:25: error: use of undeclared identifier 'GL_MIN_PROGRAM_TEXTURE_GATHER_OFFSET_ARB'
        gl->GetIntegerv(GL_MIN_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &min_gather_offset);
                        ^
../video/out/opengl/ra_gl.c:128:25: error: use of undeclared identifier 'GL_MAX_PROGRAM_TEXTURE_GATHER_OFFSET_ARB'
        gl->GetIntegerv(GL_MAX_PROGRAM_TEXTURE_GATHER_OFFSET_ARB, &max_gather_offset);
                        ^
3 errors generated.

And here's the report you wanted https://opengles.gpuinfo.org/displayreport.php?id=7310

kasper93 commented 4 months ago

And here's the report you wanted https://opengles.gpuinfo.org/displayreport.php?id=7310

Thanks, gather seem to be supported, so not sure what can we do. I see that sfan5 is looking into it, so wait for him to comment.

sfan5 commented 4 months ago

I was actually just investigating libplacebo's feature checks, but ok fine:

your gpu report: offset from -32 to 31 the log does not contain a shader dump but we can figure out that ewa_lanczos has a radius_cutoff of 3.073225. this means none of the involved offsets will be bigger than -4 or 4. => vo_gpu is doing it correctly but it's not working => vendor driver bug

Of course I skipped the part where I pour hours into tracking this down to be 100% sure that it's the vendors fault. That's left as an exercise to the reader.

haasn commented 4 months ago

@haasn: Is this expected that libplacebo fails to get gpu caps? While other software it working?

Some people have raised these errors before but I've never managed to reproduce it or figure out which exact call raises the error.