diff mbox

[FFmpeg-devel,3/7] Removing some debugging

Message ID 20171002172439.65628-4-bjorn@giphy.com
State Superseded
Headers show

Commit Message

Bjorn Roche Oct. 2, 2017, 5:24 p.m. UTC
From: Bjorn Roche <bjorn@xowave.com>

---
 libavfilter/vf_paletteuse.c | 65 ++++++++-------------------------------------
 1 file changed, 11 insertions(+), 54 deletions(-)

Comments

Carl Eugen Hoyos Oct. 2, 2017, 6:13 p.m. UTC | #1
2017-10-02 19:24 GMT+02:00 Bjorn Roche <bjorn@giphy.com>:
> From: Bjorn Roche <bjorn@xowave.com>

> -    printf( "alpha 1, 2: %d, %d\n", c1[0], c2[0] );

Instead please merge your paletteuse patches
into one patch, I believe the new patch should
allow to encode a single RGBA frame with
transparency into pal8 png or similar.

Carl Eugen
Bjorn Roche Oct. 2, 2017, 6:44 p.m. UTC | #2
I submitted a complete patch separately (
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/216793.html), but
this is how git-email formatted things. How would you like me to proceed?

On Mon, Oct 2, 2017 at 2:13 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2017-10-02 19:24 GMT+02:00 Bjorn Roche <bjorn@giphy.com>:
> > From: Bjorn Roche <bjorn@xowave.com>
>
> > -    printf( "alpha 1, 2: %d, %d\n", c1[0], c2[0] );
>
> Instead please merge your paletteuse patches
> into one patch, I believe the new patch should
> allow to encode a single RGBA frame with
> transparency into pal8 png or similar.
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos Oct. 2, 2017, 8:19 p.m. UTC | #3
2017-10-02 20:44 GMT+02:00 Bjorn Roche <bjorn@giphy.com>:
> I submitted a complete patch separately

No.

What I wrote in my last email was (please
do not top-post here):
>> Instead please merge your paletteuse patches
>> into one patch, I believe the new patch should
>> allow to encode a single RGBA frame with
>> transparency into pal8 png or similar.

The earlier patch you sent did not only change
the paletteuse filter.

Carl Eugen
Bjorn Roche Oct. 2, 2017, 11:52 p.m. UTC | #4
On Mon, Oct 2, 2017 at 4:19 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2017-10-02 20:44 GMT+02:00 Bjorn Roche <bjorn@giphy.com>:
> > I submitted a complete patch separately
>
> No.
>
> What I wrote in my last email was (please
> do not top-post here):
> >> Instead please merge your paletteuse patches
> >> into one patch, I believe the new patch should
> >> allow to encode a single RGBA frame with
> >> transparency into pal8 png or similar.
>
> The earlier patch you sent did not only change
> the paletteuse filter.
>
>
Attached is a patch for paletteuse only. It produces broken animated gifs
when a transparent video is input, but it produces correct stills.

bjorn
Carl Eugen Hoyos Oct. 3, 2017, 8:02 a.m. UTC | #5
2017-10-03 1:52 GMT+02:00 Bjorn Roche <bjorn@giphy.com>:

> Attached is a patch for paletteuse only.

I tested the following with and without your patch:
$ ffmpeg -i fate-suite/lena.pnm -vf palettegen pal.png
$ ffmpeg -i fate-suite/lena.pnm -i pal.png -lavfi paletteuse out.png

out.png changes with your patch: Is that intended?
The input frame contains no transparency.

Your patch still contains printfs that should be removed
(change them to av_log(DEBUG) if you think they are
useful) and a new warning is shown at compilation:
libavfilter/vf_paletteuse.c: In function ‘colormap_nearest_recursive’:
libavfilter/vf_paletteuse.c:242:5: warning: ISO C90 forbids mixed
declarations and code

Please fix the style: Space after "if", no space after "if ("

Thank you, Carl Eugen
diff mbox

Patch

diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
index abee1b3735..21870c22f5 100644
--- a/libavfilter/vf_paletteuse.c
+++ b/libavfilter/vf_paletteuse.c
@@ -172,14 +172,11 @@  static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2)
     const int dg = c1[2] - c2[2];
     const int db = c1[3] - c2[3];
 
-    printf( "alpha 1, 2: %d, %d\n", c1[0], c2[0] );
-
     return ( c1[0] == 0 && c2[0] == 0 ) ? 0 : ( (c1[0] == c2[0]) ? (dr*dr + dg*dg + db*db) : (max_diff) ) ;
 }
 
 static av_always_inline uint8_t colormap_nearest_bruteforce(const uint32_t *palette, const uint8_t *rgb)
 {
-    printf( "colormap_nearest_bruteforce\n" );
     int i, pal_id = -1, min_dist = INT_MAX;
 
     for (i = 0; i < AVPALETTE_COUNT; i++) {
@@ -214,7 +211,6 @@  static void colormap_nearest_node(const struct color_node *map,
                                   const uint8_t *target,
                                   struct nearest_color *nearest)
 {
-    printf( "colormap_nearest_node\n" );
     const struct color_node *kd = map + node_pos;
     const int s = kd->split;
     int dx, nearer_kd_id, further_kd_id;
@@ -255,7 +251,6 @@  struct stack_node {
 
 static av_always_inline uint8_t colormap_nearest_iterative(const struct color_node *root, const uint8_t *target)
 {
-    printf( "colormap_nearest_iterative\n" );
     int pos = 0, best_node_id = -1, best_dist = INT_MAX, cur_color_id = 0;
     struct stack_node nodes[16];
     struct stack_node *node = &nodes[0];
@@ -266,14 +261,11 @@  static av_always_inline uint8_t colormap_nearest_iterative(const struct color_no
         const uint8_t *current = kd->val;
         const int current_to_target = diff(target, current);
 
-        printf( "%d-%d-%d-%d, %d, %d-%d-%d-%d\n", target[0], target[1], target[2], target[3], current_to_target, current[0], current[1], current[2], current[3] );
-
         /* Compare current color node to the target and update our best node if
          * it's actually better. */
         if (current_to_target < best_dist) {
             best_node_id = cur_color_id;
             if (!current_to_target) {
-                printf( "exact\n");
                 goto end; // exact match, we can return immediately
             }
             best_dist = current_to_target;
@@ -282,7 +274,6 @@  static av_always_inline uint8_t colormap_nearest_iterative(const struct color_no
         /* Check if it's not a leaf */
         if (kd->left_id != -1 || kd->right_id != -1) {
             const int split = kd->split;
-            printf( "split %d\n", split );
             const int dx = target[split-1] - current[split-1];
             int nearer_kd_id, further_kd_id;
 
@@ -317,10 +308,8 @@  static av_always_inline uint8_t colormap_nearest_iterative(const struct color_no
         /* Unstack as much as we can, typically as long as the least probable
          * branch aren't actually probable. */
         do {
-            if (--pos < 0) {
-                printf( "pos < 0\n");
+            if (--pos < 0)
                 goto end;
-            }
             node--;
         } while (node->dx2 >= best_dist);
 
@@ -351,7 +340,6 @@  static av_always_inline int color_get(struct cache_node *cache, uint32_t argb,
                                       const uint32_t *palette,
                                       const enum color_search_method search_method)
 {
-    printf("color_get\n");
     int i;
     const uint8_t argb_elts[] = {a, r, g, b};
     const uint8_t rhash = r & ((1<<NBITS)-1);
@@ -360,15 +348,11 @@  static av_always_inline int color_get(struct cache_node *cache, uint32_t argb,
     const unsigned hash = rhash<<(NBITS*2) | ghash<<NBITS | bhash;
     struct cache_node *node = &cache[hash];
     struct cached_color *e;
-    //uint32_t rgb = argb & 0xffffff;
 
     // is this transparent?
-    printf( "a: %d, ti: %d\n", a, transparency_index );
     if( a == 0 && transparency_index >= 0 ) {
-        printf( "trans %d %d %d %d\n", a, r, g, b );
         return transparency_index;
     }
-    printf( "opaque %d %d %d %d\n", a, r, g, b );
 
     for (i = 0; i < node->nb_entries; i++) {
         e = &node->entries[i];
@@ -397,7 +381,6 @@  static av_always_inline int get_dst_color_err(struct cache_node *cache,
     const uint8_t r = argb >> 16 & 0xff;
     const uint8_t g = argb >>  8 & 0xff;
     const uint8_t b = argb       & 0xff;
-    printf( "get_dst_color_err %d %d %d %d %d\n", argb, a, r, g, b );
     const int dstx = color_get(cache, argb, a, r, g, b, transparency_index, map, palette, search_method);
     const uint32_t dstc = palette[dstx];
     *er = r - (dstc >> 16 & 0xff);
@@ -412,7 +395,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                                       const enum color_search_method search_method)
 {
     int x, y;
-    printf( "set_frame\n" );
     const struct color_node *map = s->map;
     struct cache_node *cache = s->cache;
     const uint32_t *palette = s->palette;
@@ -430,7 +412,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
             int er, eg, eb;
 
             if (dither == DITHERING_BAYER) {
-                printf( "bayer\n" );
                 const int d = s->ordered_dither[(y & 7)<<3 | (x & 7)];
                 const uint8_t a8 = src[x] >> 24 & 0xff;
                 const uint8_t r8 = src[x] >> 16 & 0xff;
@@ -439,7 +420,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 const uint8_t r = av_clip_uint8(r8 + d);
                 const uint8_t g = av_clip_uint8(g8 + d);
                 const uint8_t b = av_clip_uint8(b8 + d);
-                //const uint32_t argb = a8 << 24 | r<<16 | g<<8 | b;
                 const int color = color_get(cache, src[x], a8, r, g, b, transparency_index, map, palette, search_method);
 
                 if (color < 0)
@@ -447,7 +427,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 dst[x] = color;
 
             } else if (dither == DITHERING_HECKBERT) {
-                printf( "heckbert\n" );
                 const int right = x < w - 1, down = y < h - 1;
                 const int color = get_dst_color_err(cache, src[x], map, palette, transparency_index, &er, &eg, &eb, search_method);
 
@@ -460,7 +439,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 if (right && down) src[src_linesize + x + 1] = dither_color(src[src_linesize + x + 1], er, eg, eb, 2, 3);
 
             } else if (dither == DITHERING_FLOYD_STEINBERG) {
-                printf( "fs\n" );
                 const int right = x < w - 1, down = y < h - 1, left = x > x_start;
                 const int color = get_dst_color_err(cache, src[x], map, palette, transparency_index, &er, &eg, &eb, search_method);
 
@@ -474,7 +452,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 if (right && down) src[src_linesize + x + 1] = dither_color(src[src_linesize + x + 1], er, eg, eb, 1, 4);
 
             } else if (dither == DITHERING_SIERRA2) {
-                printf( "s2\n" );
                 const int right  = x < w - 1, down  = y < h - 1, left  = x > x_start;
                 const int right2 = x < w - 2,                    left2 = x > x_start + 1;
                 const int color = get_dst_color_err(cache, src[x], map, palette, transparency_index, &er, &eg, &eb, search_method);
@@ -495,7 +472,6 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 }
 
             } else if (dither == DITHERING_SIERRA2_4A) {
-                printf( "s24a\n" );
                 const int right = x < w - 1, down = y < h - 1, left = x > x_start;
                 const int color = get_dst_color_err(cache, src[x], map, palette, transparency_index, &er, &eg, &eb, search_method);
 
@@ -508,24 +484,12 @@  static av_always_inline int set_frame(PaletteUseContext *s, AVFrame *out, AVFram
                 if (         down) src[src_linesize + x    ] = dither_color(src[src_linesize + x    ], er, eg, eb, 1, 2);
 
             } else {
-                printf( "none\n" );
                 const uint8_t a = src[x] >> 24 & 0xff;
                 const uint8_t r = src[x] >> 16 & 0xff;
                 const uint8_t g = src[x] >>  8 & 0xff;
                 const uint8_t b = src[x]       & 0xff;
                 const int color = color_get(cache, src[x], a, r, g, b, transparency_index, map, palette, search_method);
 
-                printf( "%3d,%3d,%3d,%3d -> %3d -> %3d,%3d,%3d,%3d\n",
-                    src[x] >> 24 & 0xff,
-                    src[x] >> 16 & 0xff,
-                    src[x] >>  8 & 0xff,
-                    src[x] >>  0 & 0xff,
-                    color,
-                    s->palette[color] >> 24 & 0xff,
-                    s->palette[color] >> 16 & 0xff,
-                    s->palette[color] >>  8 & 0xff,
-                    s->palette[color] >>  0 & 0xff );
-
                 if (color < 0)
                     return color;
                 dst[x] = color;
@@ -777,23 +741,16 @@  static void load_colormap(PaletteUseContext *s, int nb_colors)
             continue;
         }
         last_color = c;
-        // if ((c & 0xff000000) == 0x00000000) { // ignore totally transparent
-        //     color_used[i] = 1; // ignore transparent color(s)
-        //     continue;
-        // }
     }
 
     box.min[0] = box.min[1] = box.min[2] = 0x00;
     box.max[0] = box.max[1] = box.max[2] = 0xff;
 
-    //printf( "-0- %d\n", nb_colors );
-
     colormap_insert(s->map, color_used, &nb_used, s->palette, nb_colors, &box);
 
-    for (i = 0; i < nb_used; i++) {
-        //printf( "-p-: %d\n", s->palette[i] );
-        printf( "map: %3d %3d %3d %3d  -> %3d\n", s->map[i].val[0], s->map[i].val[1], s->map[i].val[2], s->map[i].val[3], s->map[i].palette_id );
-    }
+    // for (i = 0; i < nb_used; i++) {
+    //     printf( "map: %3d %3d %3d %3d  -> %3d\n", s->map[i].val[0], s->map[i].val[1], s->map[i].val[2], s->map[i].val[3], s->map[i].palette_id );
+    // }
 
     if (s->dot_filename)
         disp_tree(s->map, s->dot_filename);
@@ -1026,7 +983,7 @@  static void load_palette(PaletteUseContext *s, const AVFrame *palette_frame)
                 ++pal_index;
             } else {
                 has_transparency = 1;
-                printf( "ti set %d\n", pal_index );
+                //printf( "ti set %d\n", pal_index );
             }
         }
         p += p_linesize;
@@ -1036,16 +993,16 @@  static void load_palette(PaletteUseContext *s, const AVFrame *palette_frame)
 
     if( has_transparency ) {
         s->transparency_index = pal_index;
-        printf( "ti set\n" );
+        //printf( "ti set\n" );
     }
-    printf( "%d\n", s->transparency_index );
+    //printf( "%d\n", s->transparency_index );
 
 
-    for (int j=0; j<palette_frame->height * palette_frame->width; ++j ) {
-        printf( "palette: %3i: %3d, %3d, %3d, %3d\n", j, ( s->palette[j] >> 24 ) & 0xff, ( s->palette[j] >> 16 ) & 0xff, ( s->palette[j] >>  8 ) & 0xff, ( s->palette[j] >>  0 ) & 0xff );
-    }
+    // for (int j=0; j<palette_frame->height * palette_frame->width; ++j ) {
+    //     printf( "palette: %3i: %3d, %3d, %3d, %3d\n", j, ( s->palette[j] >> 24 ) & 0xff, ( s->palette[j] >> 16 ) & 0xff, ( s->palette[j] >>  8 ) & 0xff, ( s->palette[j] >>  0 ) & 0xff );
+    // }
 
-    printf( "::palette loaded with transparency index :: %d\n", s->transparency_index );
+    // printf( "::palette loaded with transparency index :: %d\n", s->transparency_index );
 
     if (!s->new)
         s->palette_loaded = 1;