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

[Ulion]

2007/12/4, Ulion <ulion2002 at gmail.com>:
> 2007/12/3, Ulion <ulion2002 at gmail.com>:
> > 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?
>
> After did more research I found the truth why the original formula like that:
>
> All these mad stuff started from the vobsub's colorspace convert bug:
> Check the source code of vobsub, you will found when vobsub convert
> from .ifo to .idx, it use codes like these: (
> http://guliverkli2.svn.sourceforge.net/viewvc/guliverkli2/src/subtitles/VobSubFile.cpp
> )
>                                 ifo->Read(&tmp, 1);
>                                 ifo->Read(&y, 1);
>                                 ifo->Read(&u, 1);
>                                 ifo->Read(&v, 1);
>
>                                 y = (y-16)*255/219;
>                                 m_pgc[i].pal[j].rgbRed = (uchar)min(max(1.0*y + 1.4022*(u-128), 0), 255);
>                                 m_pgc[i].pal[j].rgbGreen = (uchar)min(max(1.0*y - 0.3456*(u-128) -
> 0.7145*(v-128), 0), 255);
>                                 m_pgc[i].pal[j].rgbBlue = (uchar)min(max(1.0*y + 1.7710*(v-128), 0) , 255);
>
> In the first and third forumla, the red and blue were incorrectly
> calculated because use v and u in reversed places. Currect convert
> should be (according to http://www.fourcc.org/fccyvrgb.php ):
> R = Y + 1.403V'
> G = Y - 0.344U' - 0.714V'
> B = Y + 1.770U'
>
> But the mistake already happens for so long time (certainly it's a
> mistake of vobsub, check its code, it directly use palette as rgb, but
> it's not rgb), although in palette it's not real rgb values of
> palette, other apps using vobsub still have to follow the wrong
> forumlas.
>
> So they use the reversed forumlas calcuated by the vobsub wrong
> forumlas, that should be:
> y = (0.1494 * r + 0.6061 * g + 0.2445 * b ) * 255 / 219 + 16
> u = 0.6066 * r - 0.4322 * g - 0.1744 * b + 128
> v = -0.08435 * r - 0.3422 * g + 0.4266 * b + 128
>
> Check vobsub.c in mplayer, you will found it missed the '*255 / 219
> +16' part for Y,
>
> So, the correct behavior should be:
> 1. When parsing palette colors, they are not real rgb values, we
> should convert them use the reversed version of forumlas from vobsub
> official source.
> 2. When parsing custom colors, treat them as true rgb value, and use
> standard forumlas convert them to yuv.
>
> So, here first patch is for correct the Y space to the same with its
> in .ifo file.
> Second patch is convert custom colors from rgb -> yuv
> Third attachment is my test program for rgb <->yuv convert (code is
> not clean, just for test print), if interested, you can check the
> values of rgb/yuv before and after different convert. Indeed when
> doing rgb->yuv->rgb, the final result normally has some distance with
> the original value so we should avoid such convert if possible.

typo fix for second patch in last post.

-- 
Ulion
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: vobsub_cuspal_fix3.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20071204/dbfb37d0/attachment.txt>