[PATCH]lavf/gdv: Improve palette saturation
Hi! Attached patch slightly improves the saturation of the gdv palette. Please comment, Carl Eugen
On 8/26/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
Hi!
Attached patch slightly improves the saturation of the gdv palette.
Please comment, Carl Eugen
Does this match how it is displayed by original game?
2017-08-27 9:03 GMT+02:00 Paul B Mahol <onemda@gmail.com>:
On 8/26/17, Carl Eugen Hoyos <ceffmpeg@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, 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). Carl Eugen
Hi Carl, On Mon, Aug 28, 2017 at 10:49 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
2017-08-27 9:03 GMT+02:00 Paul B Mahol <onemda@gmail.com>:
On 8/26/17, Carl Eugen Hoyos <ceffmpeg@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
Am Do., 7. Sept. 2017 um 15:40 Uhr schrieb Ronald S. Bultje <rsbultje@gmail.com>:
Hi Carl,
On Mon, Aug 28, 2017 at 10:49 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
2017-08-27 9:03 GMT+02:00 Paul B Mahol <onemda@gmail.com>:
On 8/26/17, Carl Eugen Hoyos <ceffmpeg@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
2017-08-26 13:00 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
Hi!
Attached patch slightly improves the saturation of the gdv palette.
I'll push this if there are no objections. Carl Eugen
participants (3)
-
Carl Eugen Hoyos -
Paul B Mahol -
Ronald S. Bultje