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

[Ronald S. Bultje]

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?

Ronald