diff mbox

[FFmpeg-devel] delogo filter: new "uglarm" interpolation mode added

Message ID 0c15df5c-3667-c84b-e059-bc4daa0e743d@gmx.de
State Superseded
Headers show

Commit Message

Uwe Freese Dec. 27, 2018, noon UTC
Hello,

thanks for the comments. Most points were clear and I've changed the 
code accordingly (see attached new patch).

Here are the remaining questions / points to discuss:

> You can start writing it now already, because it needs to go into
> doc/filters.texi.

I've added the first version in the patch attached. Maybe some of the 
sentences have to be improved - I'm not an english native speaker.

 > The "10" should be a #define, [...]

I have now added as error handling:

av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than expected.\n");
return AVERROR(ENOMEM);

Is this ok, or how should this be implemented instead?

>> + * Copyright (c) 2019 Uwe Freese <mail@uwe-freese.de>
> Considering you authored it in 2018, this is forward-looking. ;-)

I thought it would take some days to review and when the code is 
integrated finally, it is already 2019.

Should I write 2018, 2019 instead (assuming that it won't be integrated 
before next week)?

>
>> +            for (x = logo_x1+1,
>> +                xdst = dst+logo_x1+1,
>> +                xsrc = src+logo_x1+1; x < logo_x2; x++, xdst++, xsrc++) {
> Spaces around operators: x = logo_x1 + 1
> (Also everywhere else. Unless it's the original code, then leave it be.)

The original code had no spaces around operators in this case. To be 
clear: Spaces are wanted here, right? So it should be "x = logo_x1 + 1" 
etc. right?

>
>> +    double e = 0.2 * power;
> Could power also be a double instead of an int? Would specifying a
> power of e.g. 2.5 make sense?

This is a good point. It was an int value in VirtualDub's delogo and in 
ffdshow, I think mostly because this int value was mapped to a slider in 
the GUI...

There is the multiplier of 0.2 so the differences between the parameters 
that are possible to set are small enough. Power of 2,5 is not needed 
when the 0,2 factor is used.

But the question is, if a double parameter would be preferred instead of 
an int (which is multiplied by 0,2)? The parameter to set by the user 
would be e.g. "3" or "2.2" instead of "15" or "11".

I personally would prefer int parameters a little, but to explain the 
functioning of the calculation to the user, a double might be better 
("At value 3, the weight for the consideration of a border pixel is 
distance ^ 3.").

So shall I change this and use a double parameter?

>
>> +    {"mode",    "set the interpolation mode",               OFFSET(mode), AV_OPT_TYPE_INT, { .i64 = MODE_XY},      0, 1, FLAGS, "mode"},
> min and max are MODE_XY and MODE_UGLARM (or MODE_NB-1, if you code it
> that way to give room for more modes).

I wasn't sure if one can rely on the fact that MODE_UGLARM is mapped to 
1 and is the max value, because it's not specified in the enum declaration.

But it seems we can be sure: 
https://stackoverflow.com/questions/42128376/what-is-the-rule-for-assignment-of-the-integer-value-of-enum

So I changed it.


It would be nice to get some infos and opinions about these questions. 
I'll change the code accordingly afterwards.
Also let me know if there's something else to change.

Regards,
Uwe

Comments

Moritz Barsnick Dec. 27, 2018, 3:22 p.m. UTC | #1
On Thu, Dec 27, 2018 at 13:00:29 +0100, Uwe Freese wrote:

These were just some remarks from me, still pending other, better
reviews.

>  > The "10" should be a #define, [...]
> 
> I have now added as error handling:
> 
> av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than expected.\n");
> return AVERROR(ENOMEM);
> 
> Is this ok, or how should this be implemented instead?

I'll leave it up to others to tell you how many planes there actually
are.

> > Considering you authored it in 2018, this is forward-looking. ;-)
> 
> I thought it would take some days to review and when the code is 
> integrated finally, it is already 2019.
> 
> Should I write 2018, 2019 instead (assuming that it won't be integrated 
> before next week)?

I was basically kidding. But if it goes through this year, it's wrong.
Actually, the year of actual authorship is relevant AFAIK. So if you
have reused a lot of your original code, you could write "2001, 2018".
It probably doesn't matter.

> The original code had no spaces around operators in this case. To be 
> clear: Spaces are wanted here, right? So it should be "x = logo_x1 + 1" 
> etc. right?

It was unclear, due to reindentation (and probably moving code blocks
around) that this was original code. If you retain it functionally
(even when reindenting), please don't fix its whitespace. Only make
sure your new code adheres to the correct style.

> > Could power also be a double instead of an int? Would specifying a
> > power of e.g. 2.5 make sense?
> 
> This is a good point. It was an int value in VirtualDub's delogo and in 
> ffdshow, I think mostly because this int value was mapped to a slider in 
> the GUI...
> 
> There is the multiplier of 0.2 so the differences between the parameters 
> that are possible to set are small enough. Power of 2,5 is not needed 
> when the 0,2 factor is used.
> 
> But the question is, if a double parameter would be preferred instead of 
> an int (which is multiplied by 0,2)? The parameter to set by the user 
> would be e.g. "3" or "2.2" instead of "15" or "11".
> 
> I personally would prefer int parameters a little, but to explain the 
> functioning of the calculation to the user, a double might be better 
> ("At value 3, the weight for the consideration of a border pixel is 
> distance ^ 3.").
> 
> So shall I change this and use a double parameter?

This is basically up to you to decide. Is a "floating" power target
useful or not? If it's good enough in (e.g. 15) integral steps, do
leave it that way. It's really up to you.

> >> +    {"mode",    "set the interpolation mode",               OFFSET(mode), AV_OPT_TYPE_INT, { .i64 = MODE_XY},      0, 1, FLAGS, "mode"},
> > min and max are MODE_XY and MODE_UGLARM (or MODE_NB-1, if you code it
> > that way to give room for more modes).
> 
> I wasn't sure if one can rely on the fact that MODE_UGLARM is mapped to 
> 1 and is the max value, because it's not specified in the enum declaration.
> 
> But it seems we can be sure: 
> https://stackoverflow.com/questions/42128376/what-is-the-rule-for-assignment-of-the-integer-value-of-enum
> 
> So I changed it.

That's why I mentioned the MODE_NB style. I just checked an arbitrary
other filter and found libavfilter/f_metadata.c:

enum MetadataMode {
    METADATA_SELECT,
    METADATA_ADD,
    METADATA_MODIFY,
    METADATA_DELETE,
    METADATA_PRINT,
    METADATA_NB
};

The final _NB leaves room for adding other enums, and the valid range
always remains 0 .. METADATA_NB-1. That's nice style for anything which
potentially expands, but it's not mandatory.

> Also let me know if there's something else to change.

Yes, please get some more reviews on this.

> Subject: [PATCH] Add new delogo interpolation mode uglarm.

Commit message style: "avfilter/vf_delogo: add uglarm interpolation mode"

> +@item mode
> +Specify the interpolation mode used to remove the logo. 'xy' uses
> +only the border pixels in straight x and y direction to replace any
> +logo pixel. It tends to flicker and to show horizontal and vertical
> +lines. 'uglarm' considers the whole border for every logo pixel to
> +replace. It uses the power of the distance to any border pixel as
> +weight to which amount it's taken into account. This results in a
> +more blurred area, which tends to be less distracting. The default
> +value is 'xy'.

Use @var{xy} and @var{uglarm}.
"it's" -> "it is" preferred.

You could also use the format:
@item mode
Intro text.

It accepts the following values:
@table @option
@item xy
This mode uses...
This mode is the default.
@item uglarm
This mode considers...
@end table

> +@item power
> +Specify the power (factor) used to calculate the weight out of the
> +distance in 'uglarm' mode (weight = distance ^ 0.2 * power). The
> +value 0 results in a logo area which has the same average color
> +everywhere. The higher the value, the more relevant a near border
> +pixel will get, meaning that the borders of the logo area are more
> +similar to the surrounding pixels. The default value 15 results in
> +power 3 (= 0.2 * 15).

Otherwise okay.

> +    if (s->mode == MODE_UGLARM)
> +    {

if ( ... ) {

No further comments from me. The patch applies, the filter works, and
it's only slightly slower than the xy mode. I couldn't find any good
material to compare the result thouh. Good work anyway!

Cheers,
Moritz
Carl Eugen Hoyos Dec. 27, 2018, 7:25 p.m. UTC | #2
2018-12-27 13:00 GMT+01:00, Uwe Freese <uwe.freese@gmx.de>:

>  > The "10" should be a #define, [...]
>
> I have now added as error handling:
>
> av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than expected.\n");
> return AVERROR(ENOMEM);
>
> Is this ok, or how should this be implemented instead?

Not sure I understand: How can plane get >= MAX_PLANES?
If this is impossible (as I believe), please use av_assert0().

Please use av_freep() instead of av_free() for non-local
variables (the parameter needs an additional "&").

Carl Eugen
Uwe Freese Dec. 27, 2018, 9:02 p.m. UTC | #3
Hello,

Am 27.12.18 um 20:25 schrieb Carl Eugen Hoyos:
>
>> I have now added as error handling:
>>
>> av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than expected.\n");
>> return AVERROR(ENOMEM);
>>
>> Is this ok, or how should this be implemented instead?
> Not sure I understand: How can plane get >= MAX_PLANES?
> If this is impossible (as I believe), please use av_assert0().

I meant the use of "ENOMEM" and if there's a better error constant to 
use here.

At this line, the error is not about memory, but that the video input 
format is unexpected. Maybe there is a better value to use here. I 
didn't find where these error constants are defined.

> Please use av_freep() instead of av_free() for non-local
> variables (the parameter needs an additional "&").

The doc said that av_freep was recommended, but I thought, when I call 
av_free at uninit, the possibility that a dangling reference is used is 
zero.

OK, no problem, I can change this in the next version (tomorrow). But if 
av_freep should be used "everywhere", maybe the docs should also be 
changed? At least it wasn't clear to me that even in uninit, av_freep 
would be preferred.

Regards,
Uwe
Carl Eugen Hoyos Dec. 27, 2018, 11:08 p.m. UTC | #4
2018-12-27 22:02 GMT+01:00, Uwe Freese <uwe.freese@gmx.de>:

> Am 27.12.18 um 20:25 schrieb Carl Eugen Hoyos:
>>
>>> I have now added as error handling:
>>>
>>> av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than
>>> expected.\n");
>>> return AVERROR(ENOMEM);
>>>
>>> Is this ok, or how should this be implemented instead?
>>
>> Not sure I understand: How can plane get >= MAX_PLANES?
>> If this is impossible (as I believe), please use av_assert0().
>
> I meant the use of "ENOMEM" and if there's a better error
> constant to use here.
>
> At this line, the error is not about memory, but that the video input
> format is unexpected.

Is there a format with too many planes?
If not, please use an assert, do not return an error.

Carl Eugen
diff mbox

Patch

From f2c56ed60403b3bdee9749170c155720acd689c6 Mon Sep 17 00:00:00 2001
From: breaker27 <mail@uwe-freese.de>
Date: Wed, 26 Dec 2018 18:16:48 +0100
Subject: [PATCH] Add new delogo interpolation mode uglarm.

---
 doc/filters.texi        |  19 +++++
 libavfilter/vf_delogo.c | 189 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 199 insertions(+), 9 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 65ce25bc18..39bdb0d188 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -7952,6 +7952,25 @@  Specify the thickness of the fuzzy edge of the rectangle (added to
 deprecated, setting higher values should no longer be necessary and
 is not recommended.
 
+@item mode
+Specify the interpolation mode used to remove the logo. 'xy' uses
+only the border pixels in straight x and y direction to replace any
+logo pixel. It tends to flicker and to show horizontal and vertical
+lines. 'uglarm' considers the whole border for every logo pixel to
+replace. It uses the power of the distance to any border pixel as
+weight to which amount it's taken into account. This results in a
+more blurred area, which tends to be less distracting. The default
+value is 'xy'.
+
+@item power
+Specify the power (factor) used to calculate the weight out of the
+distance in 'uglarm' mode (weight = distance ^ 0.2 * power). The
+value 0 results in a logo area which has the same average color
+everywhere. The higher the value, the more relevant a near border
+pixel will get, meaning that the borders of the logo area are more
+similar to the surrounding pixels. The default value 15 results in
+power 3 (= 0.2 * 15).
+
 @item show
 When set to 1, a green rectangle is drawn on the screen to simplify
 finding the right @var{x}, @var{y}, @var{w}, and @var{h} parameters.
diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
index 065d093641..f97ef0598c 100644
--- a/libavfilter/vf_delogo.c
+++ b/libavfilter/vf_delogo.c
@@ -2,6 +2,7 @@ 
  * Copyright (c) 2002 Jindrich Makovicka <makovick@gmail.com>
  * Copyright (c) 2011 Stefano Sabatini
  * Copyright (c) 2013, 2015 Jean Delvare <jdelvare@suse.com>
+ * Copyright (c) 2019 Uwe Freese <mail@uwe-freese.de>
  *
  * This file is part of FFmpeg.
  *
@@ -25,6 +26,9 @@ 
  * A very simple tv station logo remover
  * Originally imported from MPlayer libmpcodecs/vf_delogo.c,
  * the algorithm was later improved.
+ * The "UGLARM" mode was first implemented 2001 by Uwe Freese for Virtual
+ * Dub's LogoAway filter (by Krzysztof Wojdon), taken over into ffdshow's
+ * logoaway filter (by Milan Cutka), from where it was ported to ffmpeg.
  */
 
 #include "libavutil/common.h"
@@ -50,6 +54,10 @@ 
  * @param logo_w width of the logo
  * @param logo_h height of the logo
  * @param band   the size of the band around the processed area
+ * @param *uglarmtable      pointer to weight table in UGLARM interpolation mode,
+ *                          zero when x-y mode is used
+ * @param *uglarmweightsum  pointer to weight sum table in UGLARM interpolation mode,
+ *                          zero when x-y mode is used
  * @param show   show a rectangle around the processed area, useful for
  *               parameters tweaking
  * @param direct if non-zero perform in-place processing
@@ -58,7 +66,8 @@  static void apply_delogo(uint8_t *dst, int dst_linesize,
                          uint8_t *src, int src_linesize,
                          int w, int h, AVRational sar,
                          int logo_x, int logo_y, int logo_w, int logo_h,
-                         unsigned int band, int show, int direct)
+                         unsigned int band, double *uglarmtable,
+                         double *uglarmweightsum, int show, int direct)
 {
     int x, y;
     uint64_t interp, weightl, weightr, weightt, weightb, weight;
@@ -89,6 +98,7 @@  static void apply_delogo(uint8_t *dst, int dst_linesize,
     dst += (logo_y1 + 1) * dst_linesize;
     src += (logo_y1 + 1) * src_linesize;
 
+    if (!uglarmtable) {
     for (y = logo_y1+1; y < logo_y2; y++) {
         left_sample = topleft[src_linesize*(y-logo_y1)]   +
                       topleft[src_linesize*(y-logo_y1-1)] +
@@ -151,12 +161,123 @@  static void apply_delogo(uint8_t *dst, int dst_linesize,
         dst += dst_linesize;
         src += src_linesize;
     }
+    } else {
+        int bx, by;
+        double interpd;
+
+        for (y = logo_y1 + 1; y < logo_y2; y++) {
+            for (x = logo_x1 + 1,
+                xdst = dst + logo_x1 + 1,
+                xsrc = src + logo_x1 + 1; x < logo_x2; x++, xdst++, xsrc++) {
+
+                if (show && (y == logo_y1 + 1 || y == logo_y2 - 1 ||
+                            x == logo_x1 + 1 || x == logo_x2 - 1)) {
+                    *xdst = 0;
+                    continue;
+                }
+
+                interpd = 0;
+
+                for (bx = 0; bx < logo_w; bx++) {
+                    interpd += topleft[bx] *
+                        uglarmtable[abs(bx - (x - logo_x1)) + (y - logo_y1) * (logo_w - 1)];
+                    interpd += botleft[bx] *
+                        uglarmtable[abs(bx - (x - logo_x1)) + (logo_h - (y - logo_y1) - 1) * (logo_w - 1)];
+                }
+
+                for (by = 1; by < logo_h - 1; by++) {
+                    interpd += topleft[by * src_linesize] *
+                        uglarmtable[(x - logo_x1) + abs(by - (y - logo_y1)) * (logo_w - 1)];
+                    interpd += topleft[by * src_linesize + (logo_w - 1)] *
+                        uglarmtable[logo_w - (x - logo_x1) - 1 + abs(by - (y - logo_y1)) * (logo_w - 1)];
+                }
+
+                interp = (uint64_t)(interpd /
+                    uglarmweightsum[(x - logo_x1) - 1 + (y - logo_y1 - 1) * (logo_w - 2)]);
+                *xdst = interp;
+            }
+
+            dst += dst_linesize;
+            src += src_linesize;
+        }
+    }
 }
 
+/**
+ * Calculate the lookup tables to be used in UGLARM interpolation mode.
+ *
+ * @param *uglarmtable      Pointer to table containing weights for each possible
+ *                          diagonal distance between a border pixel and an inner
+ *                          logo pixel.
+ * @param *uglarmweightsum  Pointer to a table containing the weight sum to divide
+ *                          by for each pixel within the logo area.
+ * @param sar               The sar to take into account when calculating lookup
+ *                          tables.
+ * @param logo_w            width of the logo
+ * @param logo_h            height of the logo
+ * @param power             power of uglarm interpolation
+ */
+static void calc_uglarm_tables(double *uglarmtable, double *uglarmweightsum,
+                               AVRational sar, int logo_w, int logo_h, int power)
+{
+    double e = 0.2 * power;
+    double aspect = (double)sar.num / sar.den;
+    int x, y;
+
+    /* uglarmtable will contain a weight for each possible diagonal distance
+     * between a border pixel and an inner logo pixel. The maximum distance in
+     * each direction between border and an inner pixel can be logo_w - 1. The
+     * weight of a border pixel which is x,y pixels away is stored at position
+     * x + y * (logo_w - 1). */
+    for (y = 0; y < logo_h - 1; y++)
+        for (x = 0; x < logo_w - 1; x++) {
+            if (x + y != 0) {
+                double d = pow(sqrt(x * x * aspect * aspect + y * y), e);
+                uglarmtable[x + y * (logo_w - 1)] = 1.0 / d;
+            } else {
+                uglarmtable[x + y * (logo_w - 1)] = 1.0;
+            }
+        }
+
+    /* uglarmweightsum will contain the sum of all weights which is used when
+     * an inner pixel of the logo at position x,y is calculated out of the
+     * border pixels. The aggregated value has to be divided by that. The value
+     * to use for the inner 1-based logo position x,y is stored at
+     * (x - 1) + (y - 1) * (logo_w - 2). */
+    for (y = 1; y < logo_h - 1; y++)
+        for (x = 1; x < logo_w - 1; x++) {
+            double weightsum = 0;
+
+            for (int bx = 0; bx < logo_w; bx++) {
+                /* top border */
+                weightsum += uglarmtable[abs(bx - x) + y * (logo_w - 1)];
+                /* bottom border */
+                weightsum += uglarmtable[abs(bx - x) + (logo_h - y - 1) * (logo_w - 1)];
+            }
+
+            for (int by = 1; by < logo_h - 1; by++) {
+                /* left border */
+                weightsum += uglarmtable[x + abs(by - y) * (logo_w - 1)];
+                /* right border */
+                weightsum += uglarmtable[(logo_w - x - 1) + abs(by - y) * (logo_w - 1)];
+            }
+
+            uglarmweightsum[(x - 1) + (y - 1) * (logo_w - 2)] = weightsum;
+        }
+}
+
+enum mode {
+    MODE_XY,
+    MODE_UGLARM
+};
+
+#define MAX_PLANES 10
+
 typedef struct DelogoContext {
     const AVClass *class;
-    int x, y, w, h, band, show;
-}  DelogoContext;
+    int x, y, w, h, band, mode, power, show;
+    double *uglarmtable[MAX_PLANES], *uglarmweightsum[MAX_PLANES];
+} DelogoContext;
 
 #define OFFSET(x) offsetof(DelogoContext, x)
 #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
@@ -171,6 +292,10 @@  static const AVOption delogo_options[]= {
     { "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
     { "t",    "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
 #endif
+    { "mode", "set the interpolation mode",OFFSET(mode), AV_OPT_TYPE_INT, { .i64 = MODE_XY }, MODE_XY, MODE_UGLARM, FLAGS, "mode" },
+        { "xy",     "use pixels in straight x and y direction",  OFFSET(mode), AV_OPT_TYPE_CONST, { .i64 = MODE_XY },     0, 0, FLAGS, "mode" },
+        { "uglarm", "UGLARM mode, use full border",              OFFSET(mode), AV_OPT_TYPE_CONST, { .i64 = MODE_UGLARM }, 0, 0, FLAGS, "mode" },
+    { "power","power of UGLARM interpolation", OFFSET(power), AV_OPT_TYPE_INT, { .i64 = 15 }, 0, 30, FLAGS },
     { "show", "show delogo area",          OFFSET(show), AV_OPT_TYPE_BOOL,{ .i64 =  0 },  0, 1,       FLAGS },
     { NULL }
 };
@@ -215,8 +340,8 @@  static av_cold int init(AVFilterContext *ctx)
 #else
     s->band = 1;
 #endif
-    av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n",
-           s->x, s->y, s->w, s->h, s->band, s->show);
+    av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d mode:%d power:%d show:%d\n",
+           s->x, s->y, s->w, s->h, s->band, s->mode, s->power, s->show);
 
     s->w += s->band*2;
     s->h += s->band*2;
@@ -226,6 +351,19 @@  static av_cold int init(AVFilterContext *ctx)
     return 0;
 }
 
+static av_cold void uninit(AVFilterContext *ctx)
+{
+    DelogoContext *s = ctx->priv;
+
+    if (s->mode == MODE_UGLARM)
+    {
+        for (int plane = 0; plane < MAX_PLANES; plane++) {
+            av_free(s->uglarmtable[plane]);
+            av_free(s->uglarmweightsum[plane]);
+        }
+    }
+}
+
 static int config_input(AVFilterLink *inlink)
 {
     DelogoContext *s = inlink->dst->priv;
@@ -274,16 +412,48 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
         int hsub = plane == 1 || plane == 2 ? hsub0 : 0;
         int vsub = plane == 1 || plane == 2 ? vsub0 : 0;
 
+        /* Up and left borders were rounded down, inject lost bits
+        * into width and height to avoid error accumulation */
+        int logo_w = AV_CEIL_RSHIFT(s->w + (s->x & ((1<<hsub)-1)), hsub);
+        int logo_h = AV_CEIL_RSHIFT(s->h + (s->y & ((1<<vsub)-1)), vsub);
+
+        /* Init lookup tables once */
+        if (s->mode == MODE_UGLARM) {
+            if (plane >= MAX_PLANES) {
+                av_log(inlink->src, AV_LOG_ERROR, "More planes in frame than expected.\n");
+                return AVERROR(ENOMEM);
+            }
+            
+            if (!s->uglarmtable[plane]) {
+                s->uglarmtable[plane] =
+                    (double*)av_malloc((logo_w - 1) * (logo_h - 1) * sizeof(double));
+
+                if (!s->uglarmtable[plane]) {
+                    return AVERROR(ENOMEM);
+                }
+
+                s->uglarmweightsum[plane] =
+                    (double*)av_malloc((logo_w - 2) * (logo_h - 2) * sizeof(double));
+
+                if (!s->uglarmweightsum[plane]) {
+                    return AVERROR(ENOMEM);
+                }
+
+                calc_uglarm_tables(s->uglarmtable[plane],
+                                s->uglarmweightsum[plane],
+                                sar, logo_w, logo_h, s->power);
+            }
+        }
+
         apply_delogo(out->data[plane], out->linesize[plane],
                      in ->data[plane], in ->linesize[plane],
                      AV_CEIL_RSHIFT(inlink->w, hsub),
                      AV_CEIL_RSHIFT(inlink->h, vsub),
                      sar, s->x>>hsub, s->y>>vsub,
-                     /* Up and left borders were rounded down, inject lost bits
-                      * into width and height to avoid error accumulation */
-                     AV_CEIL_RSHIFT(s->w + (s->x & ((1<<hsub)-1)), hsub),
-                     AV_CEIL_RSHIFT(s->h + (s->y & ((1<<vsub)-1)), vsub),
+                     logo_w, logo_h,
                      s->band>>FFMIN(hsub, vsub),
+                     s->uglarmtable[plane],
+                     s->uglarmweightsum[plane],
                      s->show, direct);
     }
 
@@ -317,6 +487,7 @@  AVFilter ff_vf_delogo = {
     .priv_size     = sizeof(DelogoContext),
     .priv_class    = &delogo_class,
     .init          = init,
+    .uninit        = uninit,
     .query_formats = query_formats,
     .inputs        = avfilter_vf_delogo_inputs,
     .outputs       = avfilter_vf_delogo_outputs,
-- 
2.11.0