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

[Joey Parrish]

On Sun, Mar 23, 2003 at 12:41:22AM +0100, Arpi wrote:
> 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...

No, but the current way is to export the native format in an mpi that
says BGR32 no matter what it really is.  The only thing one can do with
non-BGR32 png's right now is mencoder -ovc copy, which is useless since
mplayer won't play them back properly.

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

It handles everything other than BGR32 badly.  This is the cheap fix
that involves the least amount of mplayer core changes.

> > 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

So do you have any suggestion for a better way to fix this?

> > 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.

So, theory aside, how does it work in practice?  Have you used -mf
type=png recently?  It can't make it work with either 24-bit or paletted
png's.  24-bit will start working as soon as the codecs.conf part is
applied, but then 32 will be broken and paletted will remain broken.

> > 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.

If this is the case, then why the code here:

 // (re)init libvo if image parameters changed (width/height/colorspace)
 if(last_w!=png_width || last_h!=png_height || last_c!=out_fmt){
    last_w=png_width; last_h=png_height; last_c=out_fmt;
    if(!out_fmt) return NULL;
    if(!mpcodecs_config_vo(sh,png_width,png_height,out_fmt)) return NULL;
 }

> > 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...

 From libmpcodecs.txt:
> For the vd (codec), control() with VDCTRL_QUERY_FORMAT will be
> called. If it doesn't implement VDCTRL_QUERY_FORMAT, (ie answers
> CONTROL_UNKNOWN or CONTROL_NA) it will be assumed to be CONTROL_TRUE
> (csp supported)!

> So, by default, if the list of supported colorspaces is constant,
> doesn't depend on the actual file's/stream's header, it's enough
> to list them in codecs.conf ('out' field), and don't implement
> VDCTRL_QUERY_FORMAT. It's the case for the most codecs.

> If the supported csp list depends on the file being decoded, list the
> possible out formats (colorspaces) in codecs.conf, and implement the
> VDCTRL_QUERY_FORMAT to test the availability of the given csp for the
> given video file/stream.

So, if we don't have query_format in vd_mpng, then we will always get
32-bit buffer?  Then how can 8-bit data be exported the right way?

Do we have query_format make a choice from the first frame?  If we set
up an 8bit buffer based on frame 1 and then frame 2 is 32bit, then we
will overwrite the buffer.

> 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

Since the out_fmt code doesn't do anything (we always get BGR32 buffer)
then I don't think it should be there.  It is misleading and useless.
If we mean to export native formats, then it should be fixed to do so.
Right now it is _broken_.  So is uniformity worse than no fix at all?
Should I send example images of how normal pngs are being read by
mplayer?

--Joey