[FFmpeg-devel] [PATCH]lavf/gdv: Improve palette saturation

[Carl Eugen Hoyos]

Am Do., 7. Sept. 2017 um 15:40 Uhr schrieb Ronald S. Bultje
<rsbultje at gmail.com>:
>
> Hi Carl,
>
> On Mon, Aug 28, 2017 at 10:49 AM, Carl Eugen Hoyos <ceffmpeg at gmail.com>
> wrote:
>
> > 2017-08-27 9:03 GMT+02:00 Paul B Mahol <onemda at gmail.com>:
> > > On 8/26/17, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> >
> > >> Attached patch slightly improves the saturation of the
> > >> gdv palette.
> > >>
> > >> Please comment, Carl Eugen
> > >
> > > Does this match how it is displayed by original game?
> >
> > The original game was written for a graphic adapter that
> > supports 6bit palette. FFmpeg only supports 8bit palette,
> >
>
> OK, so that matches the current code:
>
>              unsigned r = bytestream2_get_byte(gb);
>              unsigned g = bytestream2_get_byte(gb);
>              unsigned b = bytestream2_get_byte(gb);
>              gdv->pal[i] = 0xFFU << 24 | r << 18 | g << 10 | b << 2;
>
> This indeed suggests the byte only contains 6 lower bits, the upper 2 being
> 0. It also indeed suggests that "white" (111111) would be converted to (in
> bits) 11111100, not 11111111, which is indeed unsaturated.
>
>
> > the patch makes the colour white (and all saturated colours)
> > as similar to the intended output as possible and does not
> > change black (and other colours with little intensity).
>
>
> So let's say you have (in bits) 111111 or 000000 (white and black).
>
>              unsigned r = bytestream2_get_byte(gb);
>              unsigned g = bytestream2_get_byte(gb);
>              unsigned b = bytestream2_get_byte(gb);
> +            r |= r >> 4;
> +            g |= g >> 4;
> +            b |= b >> 4;
>
> This converts that to 111111 and 000000.
>
>              gdv->pal[i] = 0xFFU << 24 | r << 18 | g << 10 | b << 2;
>
> And it seems to me that the color is then _still_ 11111100, which is
> unsaturated. Don't you want to do something like:
>
> #define convert6to8(x) ((x << 2) | ((x + 8) >> 4))
>
>              unsigned r = bytestream2_get_byte(gb);
>              unsigned g = bytestream2_get_byte(gb);
>              unsigned b = bytestream2_get_byte(gb);
> +            r = convert6to8(r);
> +            g = convert6to8(g);
> +            b = convert6to8(b);
>              gdv->pal[i] = (0xFFU << 24) | (r << 16) | (g << 8) | b;
>
> Or am I misunderstanding?

No, thank you for your review!

New patch attached.

Please comment, Carl Eugen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-gdv-Improve-saturation-of-gdv-palette.patch
Type: text/x-patch
Size: 823 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200412/eea42d75/attachment.bin>