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

[Ulion]

2007/12/9, Ulion <ulion2002 at gmail.com>:
> 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.

Done.

-- 
Ulion