[MPlayer-dev-eng] [BUG] bug in vo_png?

[Joey Parrish]

On Mon, May 13, 2002 at 09:57:06PM +0200, Arpi wrote:
> Hi,
> 
> >   - segfaults when converting from 32 to 24 bit color
> >     (found after removing I420 support from vo_png [with the same video])
> should be debugged out... i cannot reproduce
> 
> > I fixed this by disabling both I420 and RGB32 support in vo_png.  MPlayer
> > now converts the colorspace properly before sending it to vo_png.
> it is not fix... it's not even a workaround...
> anyway iyuv==i420, you removed i420 byt left iyuv...
> and iyuv==i420==yv12 in the viewpoint of mplayer code
> so remove all not only 1 of 3
> but do not remove all, rather fix them or at leats debug why does it crash

Well, I'm not sure exactly what the right thing to do is.
Here's a series of changes to vo_png and the errors they produce.

First, using mplayer -vo png i263.avi we get things like this:
http://joey.yunamusic.com/mplayer/bad_color.png  (the sky should be blue)

So, I assume that the colorspace conversion inside the plugin is bad,
and make this change to find out (forgive the non-attached patches):

--- main/libvo/vo_png.c Thu Feb 28 22:17:05 2002
+++ main.dev/libvo/vo_png.c     Mon May 13 16:56:42 2002
@@ -315,9 +315,9 @@
 query_format(uint32_t format)
 {
     switch(format){
-    case IMGFMT_IYUV:
-    case IMGFMT_I420:
-    case IMGFMT_YV12:
+//    case IMGFMT_IYUV:
+//    case IMGFMT_I420:
+//    case IMGFMT_YV12:
     case IMGFMT_RGB|24:
     case IMGFMT_BGR|24:
     case IMGFMT_BGR|32:

And get this output from gdb (after recompiling with --enable-debug=3):

Start playing...
*** [vo] Allocating mp_image_t, 352x240x32bpp RGB packed, 337920 bytes
*** [vo] Allocating mp_image_t, 352x240x32bpp RGB packed, 337920 bytes
V:   0.1    3   0%  0%  0.0% 0 0 0%
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 18105)]
0x1001a9c5 in ?? () at eval.c:41
41      eval.c: No such file or directory.
(gdb) bt
#0  0x1001a9c5 in ?? () at eval.c:41
Cannot access memory at address 0x0
(gdb)

This seems like a very uninformative backtrace.
By using this patch:

--- main/libvo/vo_png.c Thu Feb 28 22:17:05 2002
+++ main.dev/libvo/vo_png.c     Mon May 13 17:09:51 2002
@@ -226,8 +226,8 @@

     if (image_format == IMGFMT_BGR32)
     {
-      rgb32to24(src[0], image_data, image_width * image_height * 4);
-      src[0] = image_data;
+//      rgb32to24(src[0], image_data, image_width * image_height * 4);
+//      src[0] = image_data;
     }

     png = create_png(buf);
@@ -315,9 +315,9 @@
 query_format(uint32_t format)
 {
     switch(format){
-    case IMGFMT_IYUV:
-    case IMGFMT_I420:
-    case IMGFMT_YV12:
+//    case IMGFMT_IYUV:
+//    case IMGFMT_I420:
+//    case IMGFMT_YV12:
     case IMGFMT_RGB|24:
     case IMGFMT_BGR|24:
     case IMGFMT_BGR|32:

We get really odd images like this one:
http://joey.yunamusic.com/mplayer/hacked_color.png
Note that there's no segfault this time.
This leads me to believe that there's something wrong 
with the rgb32to24 line.  I don't have a real good
fix for it, though.

But using this patch to simply disable 32bit colorspace
and let MPlayer do the conversions:

--- main/libvo/vo_png.c Thu Feb 28 22:17:05 2002
+++ main.dev/libvo/vo_png.c     Mon May 13 17:03:16 2002
@@ -315,12 +315,12 @@
 query_format(uint32_t format)
 {
     switch(format){
-    case IMGFMT_IYUV:
-    case IMGFMT_I420:
-    case IMGFMT_YV12:
+//    case IMGFMT_IYUV:
+//    case IMGFMT_I420:
+//    case IMGFMT_YV12:
     case IMGFMT_RGB|24:
     case IMGFMT_BGR|24:
-    case IMGFMT_BGR|32:
+//    case IMGFMT_BGR|32:
         return 1;
     }
     return 0;

I get no crashes and images like this one:
http://joey.yunamusic.com/mplayer/good_color.png  (the sky IS blue.)

This may not be the best way to get there,
but it's all I've come up with so far.

Can anyone else reproduce these results?
I'm running RedHat 7.0/7.2, a 2.4.18 kernel,
glibc 2.2.4 (rpm package glibc-2.2.4-24).

Thanks,
--Joey

-- 

"Yes." -- John F. Kennedy