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

Submitted by Carl Eugen Hoyos on Aug. 26, 2017, 11 a.m.

Details

Message ID CAB0OVGrbWKNrJX0ab2-AR3CDiK2yPGhqK37itEw7vujxSWu2bQ@mail.gmail.com
State New
Headers show

Commit Message

Carl Eugen Hoyos Aug. 26, 2017, 11 a.m.
Hi!

Attached patch slightly improves the saturation of the gdv palette.

Please comment, Carl Eugen

Comments

Paul B Mahol Aug. 27, 2017, 7:03 a.m.
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?
Carl Eugen Hoyos Aug. 28, 2017, 2:49 p.m.
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
Carl Eugen Hoyos Sept. 7, 2017, 12:57 p.m.
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
Ronald S. Bultje Sept. 7, 2017, 1:32 p.m.
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

Patch hide | download patch | download mbox

From bbc00affb218ebe2b8074fea514bf25bbca5f890 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Sat, 26 Aug 2017 12:59:11 +0200
Subject: [PATCH] lavf/gdv: Improve saturation of gdv palette.

---
 libavcodec/gdv.c  |    3 +++
 libavformat/gdv.c |    3 +++
 2 files changed, 6 insertions(+)

diff --git a/libavcodec/gdv.c b/libavcodec/gdv.c
index dc91869..009aace 100644
--- a/libavcodec/gdv.c
+++ b/libavcodec/gdv.c
@@ -433,6 +433,9 @@  static int gdv_decode_frame(AVCodecContext *avctx, void *data,
             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;
             gdv->pal[i] = 0xFFU << 24 | r << 18 | g << 10 | b << 2;
         }
         break;
diff --git a/libavformat/gdv.c b/libavformat/gdv.c
index 3220932..98b073d 100644
--- a/libavformat/gdv.c
+++ b/libavformat/gdv.c
@@ -140,6 +140,9 @@  static int gdv_read_header(AVFormatContext *ctx)
             unsigned r = avio_r8(pb);
             unsigned g = avio_r8(pb);
             unsigned b = avio_r8(pb);
+            r |= r >> 4;
+            g |= g >> 4;
+            b |= b >> 4;
             gdv->pal[i] = 0xFFU << 24 | r << 18 | g << 10 | b << 2;
         }
     }
-- 
1.7.10.4