diff mbox

[FFmpeg-devel] palettegen: Fill with last color, not black

Message ID 1547642420.30920.1.camel@acc.umu.se
State Superseded
Headers show

Commit Message

Tomas Härdin Jan. 16, 2019, 12:40 p.m. UTC
Hi

I was helping the fine folks at peppercarrot.com with web video
nonsense, and I notice palettegen outputs more colors than it should
due to padding the generated palette with pure black.

Compare this (ffmpeg version 3.2.12-1~deb9u1):
http://www.härdin.se/files/peppercarrot_gif/output-blackspecks-64.gif
with this (282a471 with this patch applied):
http://www.härdin.se/files/peppercarrot_gif/output-fixed-64.gif

The attached patch fixes this by padding with the last color instead of
black.

/Tomas

Comments

Carl Eugen Hoyos Jan. 16, 2019, 9:16 p.m. UTC | #1
2019-01-16 13:40 GMT+01:00, Tomas Härdin <tjoppen@acc.umu.se>:
> Hi
>
> I was helping the fine folks at peppercarrot.com with web video
> nonsense, and I notice palettegen outputs more colors than it should
> due to padding the generated palette with pure black.
>
> Compare this (ffmpeg version 3.2.12-1~deb9u1):
> http://www.härdin.se/files/peppercarrot_gif/output-blackspecks-64.gif
> with this (282a471 with this patch applied):
> http://www.härdin.se/files/peppercarrot_gif/output-fixed-64.gif
>
> The attached patch fixes this by padding with the last color instead of
> black.

lgtm, but I believe there is a maintainer so please wait a few days.

Carl Eugen
Clément Bœsch Jan. 17, 2019, 8:44 a.m. UTC | #2
On Wed, Jan 16, 2019 at 01:40:20PM +0100, Tomas Härdin wrote:
> Hi
> 
> I was helping the fine folks at peppercarrot.com with web video
> nonsense, and I notice palettegen outputs more colors than it should
> due to padding the generated palette with pure black.
> 
> Compare this (ffmpeg version 3.2.12-1~deb9u1):
> http://www.härdin.se/files/peppercarrot_gif/output-blackspecks-64.gif
> with this (282a471 with this patch applied):
> http://www.härdin.se/files/peppercarrot_gif/output-fixed-64.gif
> 
> The attached patch fixes this by padding with the last color instead of
> black.
> 
> /Tomas

> From 3a01f62fdcc95cc7afaf5aa6e439b8742cce43bc Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tjoppen@acc.umu.se>
> Date: Wed, 16 Jan 2019 13:07:48 +0100
> Subject: [PATCH] palettegen: Fill with last color, not black
> 
> If we fill with black then the generated palette will have one color more
> than what the user requested. This also resulted in unwanted black specks in
> the output of paletteuse, especially when generating small palettes.
> ---
>  libavfilter/vf_palettegen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
> index 5ff73e6b2b..44323782d2 100644
> --- a/libavfilter/vf_palettegen.c
> +++ b/libavfilter/vf_palettegen.c
> @@ -245,7 +245,7 @@ static void write_palette(AVFilterContext *ctx, AVFrame *out)
>                      av_log(ctx, AV_LOG_WARNING, "Dupped color: %08"PRIX32"\n", pal[x]);
>                  last_color = pal[x];
>              } else {
> -                pal[x] = 0xff000000; // pad with black
> +                pal[x] = last_color; // pad with last color
>              }
>          }
>          pal += pal_linesize;

Code LGTM, thanks

No FATE change?

Regards,
diff mbox

Patch

From 3a01f62fdcc95cc7afaf5aa6e439b8742cce43bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tjoppen@acc.umu.se>
Date: Wed, 16 Jan 2019 13:07:48 +0100
Subject: [PATCH] palettegen: Fill with last color, not black

If we fill with black then the generated palette will have one color more
than what the user requested. This also resulted in unwanted black specks in
the output of paletteuse, especially when generating small palettes.
---
 libavfilter/vf_palettegen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
index 5ff73e6b2b..44323782d2 100644
--- a/libavfilter/vf_palettegen.c
+++ b/libavfilter/vf_palettegen.c
@@ -245,7 +245,7 @@  static void write_palette(AVFilterContext *ctx, AVFrame *out)
                     av_log(ctx, AV_LOG_WARNING, "Dupped color: %08"PRIX32"\n", pal[x]);
                 last_color = pal[x];
             } else {
-                pal[x] = 0xff000000; // pad with black
+                pal[x] = last_color; // pad with last color
             }
         }
         pal += pal_linesize;
-- 
2.11.0