[MPlayer-dev-eng] [PATCH] vobsub custom colors rgb->yuv fix.

[Ulion]

2007/12/3, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Mon, Dec 03, 2007 at 03:43:25PM +0800, Ulion wrote:
> > 2007/12/3, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > On Mon, Dec 03, 2007 at 11:31:27AM +0800, Ulion wrote:
> > > > 2007/12/2, Ulion <ulion2002 at gmail.com>:
> > > > > Hello,
> > > > >
> > > > > the custom colors in .idx file, is same with palette in .idx, all in
> > > > > rgb format, spudec all use them as yuv format, and in vobsub.c,
> > > > > palette has rgb->yuv convert code, but custom palette was not
> > > > > converted.
> > > > > Although spudec only use the 'y' part of palette, this bug will still
> > > > > make spudec use wrong 'y' value of custom palette. Here's the fix, if
> > > > > there are no objections, I will commit it tomorrow.
> > > >
> > > > Done.
> > > >
> > > > Same problem also happens in demux_mkv.c, here's a patch for it, if
> > > > there's no objections, I will commit it tomorrow.
> > >
> > > Are you really sure about this? I thought there was some reason it was
> > > like this...
> >
> > I'm sure because spudec use the second byte of the cuspal, it thought
> > it was 'Y'.
>
> Actually I meant if you have checked the specification or a different
> implementation that the mkv file does not actually store the yuv values.
> I had assumed it was intentional since the conversion was only missing
> in that case.

I checked mkv specification, it just put the .idx file content into
codec private data (remove comments, timestamps, etc). So it should be
totally same with vobsub's custom colors.
And all vobsub and mkv stored vobsub all use rgb as palette and custom
color's presentation.
But spudec all use them as yuv values. (by now only y get used)

>
> > But just now I fount the formulae has problem to convert from rgb->yuv.
> > Indeed I should use macro RGB_TO_Y_CCIR, RGB_TO_U_CCIR, RGB_TO_V_CCIR
> > to get the yuv in correct yuv color space range, should I, Reimar?
>
> You can't use stuff from libavcodec unconditionally. Also we do not
> really care about full or non-full range elsewhere so I'd consider the
> current formula "good enough" for now.

Even with libavcodec disabled, the file libavcodec/colorspace.h is
still usable because it's just group of macroes, could it be
acceptable?
And the current formula is very strange with the matrix values, I
never see these values any otherelse, and it even far away from the
non-full range formula. Are you sure current formula is correct?

> But as I can see this would be the fourth case or so where that exactly
> same code is use, I'd say it is high time to make it an extra function.

Problem is where to put it, any advice? I still prefer use libavcodec
macroes if it could.


-- 
Ulion