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

[Ulion]

2007/12/8, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Sat, Dec 08, 2007 at 10:32:05PM +0800, Ulion wrote:
> > 2007/12/8, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > On Sat, Dec 08, 2007 at 08:37:51PM +0800, Ulion wrote:
> > > [...]
> > > > +unsigned int vobsub_rgb_to_yuv(unsigned int rgb)
> > > > +{
> > > > +    int r, g, b, y, u, v;
> > > > +    r = rgb >> 16 & 0xff;
> > > > +    g = rgb >> 8 & 0xff;
> > > > +    b = rgb & 0xff;
> > > > +    y = round(( 0.299   * r + 0.587   * g + 0.114   * b) * 219 / 255 + 16);
> > > > +    u = round((-0.16874 * r - 0.33126 * g + 0.5     * b) * 224 / 255 + 128);
> > > > +    v = round(( 0.5     * r - 0.41869 * g - 0.08131 * b) * 224 / 255 + 128);
> > > > +    return y << 16 | u << 8 | v;
> > > > +}
> > >
> > > The *219/255 etc. is a really bad idea IMO, since it means the subtitles
> > > can at best become a muddy gray and never truly white.
> > > This may be acceptable or even good for videos, but not for subtitles.
> >
> > I don't think so. Value 235 of Y is truly white, values above 235 is
> > lighter than white.
> > Or value 255 of Y is truly white? Anyone make it clear?
>
> That depends on the video that is playing, MPlayer does not correctly
> distinguish it yet.
> Well, it is easily changed if it ever bothers anyone, and like this we
> don't have to bother with overflow checking, so apply it as is if you
> want.
>
> >  static int
> >  vobsub_parse_custom_colors (sh_sub_t *sh, const char *start)
> >  {
> > -  int use_custom_colors, i;
> > +  int use_custom_colors, i, tmp;
> >
> >    use_custom_colors = 0;
> >    start += 14;
> > @@ -361,8 +361,9 @@
> >           start++;
> >         for (i = 0; i < 4; i++)
> >           {
> > -           if (sscanf(start, "%06x", &sh->colors[i]) != 1)
> > +           if (sscanf(start, "%06x", &tmp) != 1)
> >               break;
> > +           sh->colors[i] = vobsub_rgb_to_yuv(tmp);
>
> I think it would be better to declare tmp in this block,
> since it is not used outside of it.
> Also for consistency making it "unsigned" instead of "int" might be
> better.

Sure, then I will commit it in tomorrow.


-- 
Ulion