mpv-player / mpv

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

RGB565 video fails to play with opengl-pbo=yes #5097

Closed CounterPillow closed 7 years ago

CounterPillow commented 7 years ago

mpv version and platform

$ mpv --version
mpv 0.27.0-339-ge6a68e2330 (C) 2000-2017 mpv/MPlayer/mplayer2 projects                       
 built on Fri Nov  3 02:12:09 CET 2017                                                       
ffmpeg library versions:                                                                     
   libavutil       56.0.100                                                                  
   libavcodec      58.2.100                                                                  
   libavformat     58.0.102                                                                  
   libswscale      5.0.101                                                                   
   libavfilter     7.0.101                                                                   
   libswresample   3.0.101                                                                   
ffmpeg version: N-88540-gb1237e2b2b

Linux 4.13.9, mesa 17.2.4, Intel GPU

Reproduction steps

  1. Play back the sample file with --no-config --opengl-pbo=yes
  2. Compare to just --no-config

Expected behavior

Video plays

Actual behavior

OpenGL errors occur, no video plays

Log file

https://0x0.st/szkA.log

Sample files

https://0x0.st/szkN.AVI

which is from https://hackaday.io/project/27427-camera-badge-for-supercon-2017 (avi_rgb565.AVI)

ghost commented 7 years ago

Works fine on nvidia.

CounterPillow commented 7 years ago

According to #intel-gfx on freenode:

<Kayden> CounterPillow: sure looks like an app bug to me
<Kayden> CounterPillow: they're calling glBufferData(GL_PIXEL_UNPACK_BUFFER, -24576, 0, GL_STREAM_DRAW)
<Kayden> size of -24576 (< 0) is causing a GL error...
<Kayden> then, glTexSubImage2D dies because the PBO is sized wrong
<Kayden> so, no image

maybe nvidia accepts values lower than 0 as 0.

kaydenl commented 7 years ago

I'm seeing: glBufferData(GL_PIXEL_UNPACK_BUFFER, -24576, 0, GL_STREAM_DRAW) which raises a GL_INVALID_VALUE error due to the negative size, and fails to create a proper PBO, so glTexSubImage2D later fails with an out of bounds PBO access, and you get no image.

I don't think you mean to be supplying a negative size...

ghost commented 7 years ago

Oh, turns out I was testing the wrong thing. Can reproduce on nvidia.

CounterPillow commented 7 years ago

so, @haasn suggests this

diff --git a/video/out/gpu/video.c b/video/out/gpu/video.c
index 9cc889b401..f1c73f3797 100644
--- a/video/out/gpu/video.c
+++ b/video/out/gpu/video.c
@@ -3268,7 +3268,7 @@ static bool pass_upload_image(struct gl_video *p, struct mp_image *mpi, uint64_t
             .tex = plane->tex,
             .src = mpi->planes[n],
             .invalidate = true,
-            .stride = mpi->stride[n],
+            .stride = abs(mpi->stride[n]),
         };

         struct dr_buffer *mapped = gl_find_dr_buffer(p, mpi->planes[n]);

while it gets rid of the opengl errors, it does produce a really messed up picture

ghost commented 7 years ago

I have a workaround queued. It was probably broken by haasn's previous refactor to this code.