Re: [FFmpeg-devel] [PATCH v2 02/32] avfilter/palette{gen, use}: revert support palettes with alpha
Please add alpha support back. On Wed, Dec 28, 2022 at 12:18 AM Clément Bœsch wrote: > This reverts commit dea673d0d548c864ec85f9260d8900d944ef7a2a. > > This change cannot work for several reasons, the most obvious ones are: > > - the alpha is being part of the scoring of the color difference, even > though we can not interpret the alpha as part of the perception of the > color (we don't even know if it's premultiplied or postmultiplied) > - the colors are averaged with their alpha value which simply cannot > work > > The command proposed in the original thread of the patch actually > produces a completely broken file: > > ffmpeg -y -loglevel verbose -i fate-suite/apng/o_sample.png > -filter_complex > "split[split1][split2];[split1]palettegen=max_colors=254:use_alpha=1[pal1];[split2][pal1]paletteuse=use_alpha=1" > -frames:v 1 out.png > > We can see that many color pixels are off, but more importantly some > colors have a random alpha value: https://imgur.com/eFQ2UK7 > > I don't see any easy fix for this unfortunately, the approach appears to > be flawed by design. > --- > doc/filters.texi| 8 -- > libavfilter/vf_palettegen.c | 127 +++- > libavfilter/vf_paletteuse.c | 225 +++- > 3 files changed, 138 insertions(+), 222 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 9b866de5ae..f51623d16a 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -18474,9 +18474,6 @@ Compute new histogram for each frame. > @end table > > Default value is @var{full}. > -@item use_alpha > -Create a palette of colors with alpha components. > -Setting this, will automatically disable 'reserve_transparent'. > @end table > > The filter also exports the frame metadata @code{lavfi.color_quant_ratio} > @@ -18555,11 +18552,6 @@ will be treated as completely opaque, and values > below this threshold will be > treated as completely transparent. > > The option must be an integer value in the range [0,255]. Default is > @var{128}. > - > -@item use_alpha > -Apply the palette by taking alpha values into account. Only useful with > -palettes that are containing multiple colors with alpha components. > -Setting this will automatically disable 'alpha_treshold'. > @end table > > @subsection Examples > diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c > index c03f62b942..bea3292796 100644 > --- a/libavfilter/vf_palettegen.c > +++ b/libavfilter/vf_palettegen.c > @@ -59,7 +59,7 @@ enum { > }; > > #define NBITS 5 > -#define HIST_SIZE (1<<(4*NBITS)) > +#define HIST_SIZE (1<<(3*NBITS)) > > typedef struct PaletteGenContext { > const AVClass *class; > @@ -67,7 +67,6 @@ typedef struct PaletteGenContext { > int max_colors; > int reserve_transparent; > int stats_mode; > -int use_alpha; > > AVFrame *prev_frame;// previous frame used for > the diff stats_mode > struct hist_node histogram[HIST_SIZE]; // histogram/hashtable of the > colors > @@ -89,7 +88,6 @@ static const AVOption palettegen_options[] = { > { "full", "compute full frame histograms", 0, AV_OPT_TYPE_CONST, > {.i64=STATS_MODE_ALL_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" }, > { "diff", "compute histograms only for the part that differs from > previous frame", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_DIFF_FRAMES}, > INT_MIN, INT_MAX, FLAGS, "mode" }, > { "single", "compute new histogram for each frame", 0, > AV_OPT_TYPE_CONST, {.i64=STATS_MODE_SINGLE_FRAMES}, INT_MIN, INT_MAX, > FLAGS, "mode" }, > -{ "use_alpha", "create a palette including alpha values", > OFFSET(use_alpha), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS }, > { NULL } > }; > > @@ -115,16 +113,15 @@ static int cmp_##name(const void *pa, const void > *pb) \ > { \ > const struct color_ref * const *a = pa; \ > const struct color_ref * const *b = pb; \ > -return (int)((*a)->color >> (8 * (3 - (pos))) & 0xff) \ > - - (int)((*b)->color >> (8 * (3 - (pos))) & 0xff); \ > +return (int)((*a)->color >> (8 * (2 - (pos))) & 0xff) \ > + - (int)((*b)->color >> (8 * (2 - (pos))) & 0xff); \ > } > > -DECLARE_CMP_FUNC(a, 0) > -DECLARE_CMP_FUNC(r, 1) > -DECLARE_CMP_FUNC(g, 2) > -DECLARE_CMP_FUNC(b, 3) > +DECLARE_CMP_FUNC(r, 0) > +DECLARE_CMP_FUNC(g, 1) > +DECLARE_CMP_FUNC(b, 2) > > -static const cmp_func cmp_funcs[] = {cmp_a, cmp_r, cmp_g, cmp_b}; > +static const cmp_func cmp_funcs[] = {cmp_r, cmp_g, cmp_b}; > > /** > * Simple color comparison for sorting the final palette > @@ -146,17 +143,6 @@ static av_always_inline int diff(const uint32_t a, > const uint32_t b) > return dr*dr + dg*dg + db*db; > } > > -static av_always_inline int diff_alpha(const uint32_t a, const uint32_t b) > -{ > -const uint8_t c1[] = {a >> 24 & 0xff, a >> 16 & 0xff, a >> 8 & 0xff, > a & 0xff}; > -const uint8_t c2[]
[FFmpeg-devel] [PATCH v2 02/32] avfilter/palette{gen, use}: revert support palettes with alpha
This reverts commit dea673d0d548c864ec85f9260d8900d944ef7a2a. This change cannot work for several reasons, the most obvious ones are: - the alpha is being part of the scoring of the color difference, even though we can not interpret the alpha as part of the perception of the color (we don't even know if it's premultiplied or postmultiplied) - the colors are averaged with their alpha value which simply cannot work The command proposed in the original thread of the patch actually produces a completely broken file: ffmpeg -y -loglevel verbose -i fate-suite/apng/o_sample.png -filter_complex "split[split1][split2];[split1]palettegen=max_colors=254:use_alpha=1[pal1];[split2][pal1]paletteuse=use_alpha=1" -frames:v 1 out.png We can see that many color pixels are off, but more importantly some colors have a random alpha value: https://imgur.com/eFQ2UK7 I don't see any easy fix for this unfortunately, the approach appears to be flawed by design. --- doc/filters.texi| 8 -- libavfilter/vf_palettegen.c | 127 +++- libavfilter/vf_paletteuse.c | 225 +++- 3 files changed, 138 insertions(+), 222 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index 9b866de5ae..f51623d16a 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -18474,9 +18474,6 @@ Compute new histogram for each frame. @end table Default value is @var{full}. -@item use_alpha -Create a palette of colors with alpha components. -Setting this, will automatically disable 'reserve_transparent'. @end table The filter also exports the frame metadata @code{lavfi.color_quant_ratio} @@ -18555,11 +18552,6 @@ will be treated as completely opaque, and values below this threshold will be treated as completely transparent. The option must be an integer value in the range [0,255]. Default is @var{128}. - -@item use_alpha -Apply the palette by taking alpha values into account. Only useful with -palettes that are containing multiple colors with alpha components. -Setting this will automatically disable 'alpha_treshold'. @end table @subsection Examples diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c index c03f62b942..bea3292796 100644 --- a/libavfilter/vf_palettegen.c +++ b/libavfilter/vf_palettegen.c @@ -59,7 +59,7 @@ enum { }; #define NBITS 5 -#define HIST_SIZE (1<<(4*NBITS)) +#define HIST_SIZE (1<<(3*NBITS)) typedef struct PaletteGenContext { const AVClass *class; @@ -67,7 +67,6 @@ typedef struct PaletteGenContext { int max_colors; int reserve_transparent; int stats_mode; -int use_alpha; AVFrame *prev_frame;// previous frame used for the diff stats_mode struct hist_node histogram[HIST_SIZE]; // histogram/hashtable of the colors @@ -89,7 +88,6 @@ static const AVOption palettegen_options[] = { { "full", "compute full frame histograms", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_ALL_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" }, { "diff", "compute histograms only for the part that differs from previous frame", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_DIFF_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" }, { "single", "compute new histogram for each frame", 0, AV_OPT_TYPE_CONST, {.i64=STATS_MODE_SINGLE_FRAMES}, INT_MIN, INT_MAX, FLAGS, "mode" }, -{ "use_alpha", "create a palette including alpha values", OFFSET(use_alpha), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS }, { NULL } }; @@ -115,16 +113,15 @@ static int cmp_##name(const void *pa, const void *pb) \ { \ const struct color_ref * const *a = pa; \ const struct color_ref * const *b = pb; \ -return (int)((*a)->color >> (8 * (3 - (pos))) & 0xff) \ - - (int)((*b)->color >> (8 * (3 - (pos))) & 0xff); \ +return (int)((*a)->color >> (8 * (2 - (pos))) & 0xff) \ + - (int)((*b)->color >> (8 * (2 - (pos))) & 0xff); \ } -DECLARE_CMP_FUNC(a, 0) -DECLARE_CMP_FUNC(r, 1) -DECLARE_CMP_FUNC(g, 2) -DECLARE_CMP_FUNC(b, 3) +DECLARE_CMP_FUNC(r, 0) +DECLARE_CMP_FUNC(g, 1) +DECLARE_CMP_FUNC(b, 2) -static const cmp_func cmp_funcs[] = {cmp_a, cmp_r, cmp_g, cmp_b}; +static const cmp_func cmp_funcs[] = {cmp_r, cmp_g, cmp_b}; /** * Simple color comparison for sorting the final palette @@ -146,17 +143,6 @@ static av_always_inline int diff(const uint32_t a, const uint32_t b) return dr*dr + dg*dg + db*db; } -static av_always_inline int diff_alpha(const uint32_t a, const uint32_t b) -{ -const uint8_t c1[] = {a >> 24 & 0xff, a >> 16 & 0xff, a >> 8 & 0xff, a & 0xff}; -const uint8_t c2[] = {b >> 24 & 0xff, b >> 16 & 0xff, b >> 8 & 0xff, b & 0xff}; -const int da = c1[0] - c2[0]; -const int dr = c1[1] - c2[1]; -const int dg = c1[2] - c2[2]; -const int db = c1[3] - c2[3]; -return da*da + dr*dr + dg*dg + db*db; -} - /** * Find the next box to split: pick the one