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

[Reimar Döffinger]

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.

Greetings,
Reimar Döffinger