[FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

[Clément Bœsch]

> Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

The commit message needs adjustment "lavfi/paletteuse: ..."

[...]
>  struct color_node {
> -    uint8_t val[3];
> +    uint8_t val[4];
>      uint8_t palette_id;
>      int split;
>      int left_id, right_id;
> @@ -86,6 +86,8 @@ typedef struct PaletteUseContext {
>      struct cache_node cache[CACHE_SIZE];    /* lookup cache */
>      struct color_node map[AVPALETTE_COUNT]; /* 3D-Tree (KD-Tree with K=3) for reverse colormap */
>      uint32_t palette[AVPALETTE_COUNT];

> +    int transparency_index; /* index in the palette of transparency. -1 if there isn't a transparency. */

"if there is no transparent color" might be more correct but I'm not a
native speaker.

> +    int trans_thresh;
>      int palette_loaded;
>      int dither;
>      int new;
> @@ -116,6 +118,7 @@ static const AVOption paletteuse_options[] = {
>      { "bayer_scale", "set scale for bayer dithering", OFFSET(bayer_scale), AV_OPT_TYPE_INT, {.i64=2}, 0, 5, FLAGS },
>      { "diff_mode",   "set frame difference mode",     OFFSET(diff_mode),   AV_OPT_TYPE_INT, {.i64=DIFF_MODE_NONE}, 0, NB_DIFF_MODE-1, FLAGS, "diff_mode" },
>          { "rectangle", "process smallest different rectangle", 0, AV_OPT_TYPE_CONST, {.i64=DIFF_MODE_RECTANGLE}, INT_MIN, INT_MAX, FLAGS, "diff_mode" },

> +    { "threshold", "set the alpha threshold for transparency. Above this threshold, pixels are considered opaque, below they are considered transparent.", OFFSET(trans_thresh), AV_OPT_TYPE_INT, {.i64=128}, 0, 255, },

The long explanation belongs in doc/filters.texi

[...]
> -static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2)
> +static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2, const int trans_thresh)
>  {
>      // XXX: try L*a*b with CIE76 (dL*dL + da*da + db*db)
> -    const int dr = c1[0] - c2[0];
> -    const int dg = c1[1] - c2[1];
> -    const int db = c1[2] - c2[2];
> -    return dr*dr + dg*dg + db*db;

> +    const static int max_diff = 195075; //equal to 255*255 + 255*255 + 255*255

That's not what I meant; I wasn't concerned about the expression but the
static const int. I was thinking "return 255*255 + 255*255 + 255*255"

[...]
>  /**
>   * Check if the requested color is in the cache already. If not, find it in the
>   * color tree and cache it.
> - * Note: r, g, and b are the component of c but are passed as well to avoid
> + * Note: a, r, g, and b are the components of argb, but are passed as well to avoid
>   * recomputing them (they are generally computed by the caller for other uses).
>   */
> -static av_always_inline int color_get(struct cache_node *cache, uint32_t color,
> -                                      uint8_t r, uint8_t g, uint8_t b,

> +static av_always_inline int color_get(struct cache_node *cache, uint32_t argb,

I'm sorry to insist, but renaming "color" belongs in another commit.

[...]
>  static av_always_inline int get_dst_color_err(struct cache_node *cache,

> -                                              uint32_t c, const struct color_node *map,
> +                                              uint32_t argb, const struct color_node *map,

ditto, "c" should be renamed in another commit

[...]
>      i = 0;
>      for (y = 0; y < palette_frame->height; y++) {
> -        for (x = 0; x < palette_frame->width; x++)
> -            s->palette[i++] = p[x];
> +        for (x = 0; x < palette_frame->width; x++) {
> +            s->palette[i] = p[x];
> +            if (p[x]>>24 < s->trans_thresh) {
> +                s->transparency_index = i; // we are assuming at most one transparent color in palette
> +            }

> +            ++i;

i++ is the appropriate idiom.

Aside from these nitpicks, I'm still concerned about how it's going to
conflict with GIF encoding where the transparent color is actually used as
a mean of not updating pixels from previous frames.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171021/23690d18/attachment.sig>