[MPlayer-dev-eng] [PATCH] mf png fix

[Arpi]

Hi,

ehh. 

> > -   case PNG_COLOR_TYPE_PALETTE:
> > -      out_fmt=IMGFMT_BGR8;
> > -      break;
> > -   case PNG_COLOR_TYPE_RGB_ALPHA:
> > -      out_fmt=IMGFMT_BGR32;
> > -      break;
> > -   case PNG_COLOR_TYPE_RGB:
> > -      out_fmt=IMGFMT_BGR24;
> > -      break;
> > -   default:
> > -      mp_msg( MSGT_DECVIDEO,MSGL_INFO,"Sorry, unsupported PNG
> colorspace: %d.\n" ,info->color_type);
> 
> This will check for paletted png input and expand it.  Then, the info
> struct is updated based on the changes.
> > + if ( info->color_type == PNG_COLOR_TYPE_PALETTE) {
> > +	 png_set_expand(png);
> > +	 png_read_update_info(png, info);
> > + }
..

so you've removed the code to export the image as-is in its native format
(paletted, bgr/bgr 24/32) and ask libpng to convert anything to bgr24.
i don't think it's the right way to fix the possible bugs...

i think it's better to fix the code if it handles grayscale or something
badly.

> Okay, so in conclusion, it's a big change.  Also, ideally we would be
> able to handle the input stream of png's in it's native format (such as

yes

> displaying 8-bit video for 8-bit input, etc.) but that leaves us with
> some questions IMHO.  If the colorspace changes, does the filter chain
> change to fit this?  From what I can tell it does not, so then we would

in theory the filter chain should support more than one config(), ie.
runtime format, resolution etc changes. most filters does work this way
but some maybe have problems, especially the last special filter, the libvo.

> have to convert them all to one format.  This means either the format of
> the first png (which may not be enough info to be true to future pngs)
> or BGR24 or BGR32.  BGR32 would be the most info, but as I said before,

afaik it's a known limitation of -mf that the images should have the same
resolution and format.

> So, I hope this doesn't get too many flames.  If you don't like what
> I've done, please explain why.  There's a lot I could learn about
> mplayer internals, but there's a lot of confusing source to read.

then read docs/tech/libmpcodecs.txt instead...

dunno about 32bit (RGBA) format, maybe it's ok to use libpng's converter
insteda of mplayer, since mplayer's RGB32/BGR32 doens't use eth alpha
channel, simply ignores it, which may be wrong for 32bit PNG files.
but for paletted and greyscale i vote for native (IMGFMT_BGR8) exporting
instead of conversion in libpng or vd_mpng.c


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu