diff mbox series

[FFmpeg-devel,v2,01/32] avfilter/palettegen: allow a minimum of 2 colors

Message ID 20221227231814.2520181-2-u@pkh.me
State Accepted
Commit cad9d7fc85bde0b30823ae7b8ce17e97b6f7a387
Headers show
Series [FFmpeg-devel,v2,01/32] avfilter/palettegen: allow a minimum of 2 colors | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Clément Bœsch Dec. 27, 2022, 11:17 p.m. UTC 
---
 libavfilter/vf_palettegen.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Tomas Härdin Dec. 28, 2022, 9:04 p.m. UTC | #1 
ons 2022-12-28 klockan 00:17 +0100 skrev Clément Bœsch:
> ---
>  libavfilter/vf_palettegen.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_palettegen.c
> b/libavfilter/vf_palettegen.c
> index 27f74fd147..c03f62b942 100644
> --- a/libavfilter/vf_palettegen.c
> +++ b/libavfilter/vf_palettegen.c
> @@ -82,7 +82,7 @@ typedef struct PaletteGenContext {
>  #define OFFSET(x) offsetof(PaletteGenContext, x)
>  #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>  static const AVOption palettegen_options[] = {
> -    { "max_colors", "set the maximum number of colors to use in the
> palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 4, 256,
> FLAGS },
> +    { "max_colors", "set the maximum number of colors to use in the
> palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 2, 256,
> FLAGS },
>      { "reserve_transparent", "reserve a palette entry for
> transparency", OFFSET(reserve_transparent), AV_OPT_TYPE_BOOL,
> {.i64=1}, 0, 1, FLAGS },
>      { "transparency_color", "set a background color for
> transparency", OFFSET(transparency_color), AV_OPT_TYPE_COLOR,
> {.str="lime"}, 0, 0, FLAGS },
>      { "stats_mode", "set statistics mode", OFFSET(stats_mode),
> AV_OPT_TYPE_INT, {.i64=STATS_MODE_ALL_FRAMES}, 0, NB_STATS_MODE-1,
> FLAGS, "mode" },
> @@ -586,6 +586,11 @@ static int init(AVFilterContext *ctx)
>      if (s->use_alpha && s->reserve_transparent)
>          s->reserve_transparent = 0;
>  
> +    if (s->max_colors - s->reserve_transparent < 2) {
> +        av_log(ctx, AV_LOG_ERROR, "max_colors=2 is only allowed
> without reserving a transparent color slot\n");

Does this mean all-black with transparent is disallowed? Shouldn't it
be max_colors + reserve < 2?

/Tomas
Clément Bœsch Dec. 28, 2022, 9:23 p.m. UTC | #2 
On Wed, Dec 28, 2022 at 10:04:26PM +0100, Tomas Härdin wrote:
[...]
> Does this mean all-black with transparent is disallowed?

Yes a single color with or without an extra transparency slot makes no
sense.

> Shouldn't it be max_colors + reserve < 2?

max_colors accounts for the transparent color as well. So if you specify
256 colors with reserve_transparent (default settings), you'll get 255
color slots and 1 for transparency.

It also means that if want to go as low as 2 colors in your palette you
can do max_colors=3 with reserve_transparent=1, or max_colors=2 with
reserve_transparent=0

  3-1 → 2
  2-0 → 2

These are the minimums.

PS: sorry for the double answer Tomas!
Tomas Härdin Jan. 3, 2023, 6:59 p.m. UTC | #3 
ons 2022-12-28 klockan 22:23 +0100 skrev Clément Bœsch:
> On Wed, Dec 28, 2022 at 10:04:26PM +0100, Tomas Härdin wrote:
> [...]
> > Does this mean all-black with transparent is disallowed?
> 
> Yes a single color with or without an extra transparency slot makes
> no
> sense.
> 
> > Shouldn't it be max_colors + reserve < 2?
> 
> max_colors accounts for the transparent color as well. So if you
> specify
> 256 colors with reserve_transparent (default settings), you'll get
> 255
> color slots and 1 for transparency.
> 
> It also means that if want to go as low as 2 colors in your palette
> you
> can do max_colors=3 with reserve_transparent=1, or max_colors=2 with
> reserve_transparent=0
> 
>   3-1 → 2
>   2-0 → 2
> 
> These are the minimums.

Right, because there's no "0bpp" pixel format with 1bpp alpha.. yet!
Something similar actually exists with GoldSrc .spr files, especially
the indexalpha mode. It is effectively "0bpp" with 8-bit alpha:
https://the303.org/tutorials/gold_sprite_p3.htm

> 
> PS: sorry for the double answer Tomas!

No problem :)

/Tomas
diff mbox series

Patch

diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
index 27f74fd147..c03f62b942 100644
--- a/libavfilter/vf_palettegen.c
+++ b/libavfilter/vf_palettegen.c
@@ -82,7 +82,7 @@  typedef struct PaletteGenContext {
 #define OFFSET(x) offsetof(PaletteGenContext, x)
 #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
 static const AVOption palettegen_options[] = {
-    { "max_colors", "set the maximum number of colors to use in the palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 4, 256, FLAGS },
+    { "max_colors", "set the maximum number of colors to use in the palette", OFFSET(max_colors), AV_OPT_TYPE_INT, {.i64=256}, 2, 256, FLAGS },
     { "reserve_transparent", "reserve a palette entry for transparency", OFFSET(reserve_transparent), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
     { "transparency_color", "set a background color for transparency", OFFSET(transparency_color), AV_OPT_TYPE_COLOR, {.str="lime"}, 0, 0, FLAGS },
     { "stats_mode", "set statistics mode", OFFSET(stats_mode), AV_OPT_TYPE_INT, {.i64=STATS_MODE_ALL_FRAMES}, 0, NB_STATS_MODE-1, FLAGS, "mode" },
@@ -586,6 +586,11 @@  static int init(AVFilterContext *ctx)
     if (s->use_alpha && s->reserve_transparent)
         s->reserve_transparent = 0;
 
+    if (s->max_colors - s->reserve_transparent < 2) {
+        av_log(ctx, AV_LOG_ERROR, "max_colors=2 is only allowed without reserving a transparent color slot\n");
+        return AVERROR(EINVAL);
+    }
+
     return 0;
 }