[FFmpeg-devel] [PATCH] avfilter/delogo: Check that logo area is inside the picture
We can only remove the logo if it is inside the picture. We need at
least one pixel around the logo area for interpolation.
Fixes ticket #5527 (Delogo crash with x=0 and/or y=0).
Signed-off-by: Jean Delvare <jdelv...@suse.com>
---
libavfilter/vf_delogo.c | 15 +++
1 file changed, 15 insertions(+)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2016-05-10 13:44:01.569842931 +0200
+++ ffmpeg/libavfilter/vf_delogo.c 2016-05-10 13:47:58.754333920 +0200
@@ -225,6 +225,20 @@ static av_cold int init(AVFilterContext
return 0;
}
+static int config_input(AVFilterLink *inlink)
+{
+DelogoContext *s = inlink->dst->priv;
+
+/* Check whether the logo area fits in the frame */
+if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > inlink->w
||
+s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > inlink->h)
{
+av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n");
+return AVERROR(EINVAL);
+}
+
+return 0;
+}
+
static int filter_frame(AVFilterLink *inlink, AVFrame *in)
{
DelogoContext *s = inlink->dst->priv;
@@ -283,6 +297,7 @@ static const AVFilterPad avfilter_vf_del
.name = "default",
.type = AVMEDIA_TYPE_VIDEO,
.filter_frame = filter_frame,
+.config_props = config_input,
},
{ NULL }
};
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/vf_delogo: change the definition of logo_x2 and logo_y2
In the code we keep using logo_x2-1 and logo_y2-1 rather than logo_x2
and logo_y2 themselves. Define them to be what we need instead, to avoid
the repeated subtractions.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
libavfilter/vf_delogo.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-12-11 15:24:55.81499 +0100
+++ ffmpeg/libavfilter/vf_delogo.c 2015-12-11 15:26:09.345968761 +0100
@@ -75,13 +75,13 @@ static void apply_delogo(uint8_t *dst, i
yclipb = FFMAX(logo_y+logo_h-h, 0);
logo_x1 = logo_x + xclipl;
-logo_x2 = logo_x + logo_w - xclipr;
+logo_x2 = logo_x + logo_w - xclipr - 1;
logo_y1 = logo_y + yclipt;
-logo_y2 = logo_y + logo_h - yclipb;
+logo_y2 = logo_y + logo_h - yclipb - 1;
-topleft = src+logo_y1 * src_linesize+logo_x1;
-topright = src+logo_y1 * src_linesize+logo_x2-1;
-botleft = src+(logo_y2-1) * src_linesize+logo_x1;
+topleft = src+logo_y1 * src_linesize+logo_x1;
+topright = src+logo_y1 * src_linesize+logo_x2;
+botleft = src+logo_y2 * src_linesize+logo_x1;
if (!direct)
av_image_copy_plane(dst, dst_linesize, src, src_linesize, w, h);
@@ -89,7 +89,7 @@ static void apply_delogo(uint8_t *dst, i
dst += (logo_y1 + 1) * dst_linesize;
src += (logo_y1 + 1) * src_linesize;
-for (y = logo_y1+1; y < logo_y2-1; y++) {
+for (y = logo_y1+1; y < logo_y2; y++) {
left_sample = topleft[src_linesize*(y-logo_y1)] +
topleft[src_linesize*(y-logo_y1-1)] +
topleft[src_linesize*(y-logo_y1+1)];
@@ -99,19 +99,19 @@ static void apply_delogo(uint8_t *dst, i
for (x = logo_x1+1,
xdst = dst+logo_x1+1,
- xsrc = src+logo_x1+1; x < logo_x2-1; x++, xdst++, xsrc++) {
+ xsrc = src+logo_x1+1; x < logo_x2; x++, xdst++, xsrc++) {
-if (show && (y == logo_y1+1 || y == logo_y2-2 ||
- x == logo_x1+1 || x == logo_x2-2)) {
+if (show && (y == logo_y1+1 || y == logo_y2-1 ||
+ x == logo_x1+1 || x == logo_x2-1)) {
*xdst = 0;
continue;
}
/* Weighted interpolation based on relative distances, taking SAR
into account */
-weightl = (uint64_t) (logo_x2-1-x) * (y-logo_y1) *
(logo_y2-1-y) * sar.den;
-weightr = (uint64_t)(x-logo_x1) * (y-logo_y1) *
(logo_y2-1-y) * sar.den;
-weightt = (uint64_t)(x-logo_x1) * (logo_x2-1-x) *
(logo_y2-1-y) * sar.num;
-weightb = (uint64_t)(x-logo_x1) * (logo_x2-1-x) * (y-logo_y1)
* sar.num;
+weightl = (uint64_t) (logo_x2-x) * (y-logo_y1) *
(logo_y2-y) * sar.den;
+weightr = (uint64_t)(x-logo_x1) * (y-logo_y1) *
(logo_y2-y) * sar.den;
+weightt = (uint64_t)(x-logo_x1) * (logo_x2-x) *
(logo_y2-y) * sar.num;
+weightb = (uint64_t)(x-logo_x1) * (logo_x2-x) * (y-logo_y1)
* sar.num;
interp =
left_sample * weightl
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
Hi Derek,
On Wed, 16 Dec 2015 17:16:26 +, Derek Buitenhuis wrote:
> On 12/15/2015 10:39 PM, Jean Delvare wrote:
> > I see two completely different things.
> >
> > The change I proposed is specific to one function, only changes const
> > vs non-const, and the object under discussion is being accessed for
> > reading only (thus the const pointer.) It would remove a memcpy but
> > does not introduce a direct copy (with =.)
>
> See below.
>
> > The change you pointed us to is touching very generic functions,
> > handling by nature a very wide variety of pointer types for both
> > reading and writing.
> >
> > So just because the change you pointed us to may be good and desirable,
> > doesn't imply that the change I am proposing is bad and undesirable.
> >
> > Side note #1: if gcc really considers pointer types that only differ by
> > "const" as different when it comes to code optimization, it is
> > seriously broken IMHO. Unfortunately the man page is quite vague on the
> > topic ("unless the types are almost the same", can you be more vague
> > please.)
>
> The C spec does. GCC happens to handle it correctly. We support a whole lot
> more compilers than GCC.
The C spec does say what pointers types are considered the same and
what pointer types should be considered different. Compilers can use
this to warn users about pointer mismatches. That's what
-Wstrict-aliasing is about.
But does the C spec say that compilers should use pointer type
differences to optimize the code by reordering the instructions? I
doubt it. This very much looks like an implementation decision on the
compiler side. That's what -fstrict-aliasing is about.
Obviously both are related as far as the compiler is concerned, as the
pointer type tracking and comparison code has to be common. These are
two different things nevertheless.
> > Side note #2: if strict-aliasing optimizations can do so much damage
> > to your code that you end up doing
> > memcpy(arg, &(void *){ NULL }, sizeof(val));
> > to set a pointer to NULL, maybe you should consider building with
> > -fno-strict-aliasing.
>
> Again, that is GCC-specific. My point is that if we *can* be correct, with
> regards to the spec, we should be. We don't gain anything, IMO, by removing
> this particular thing.
Performance, readability, consistency, is what we gain. Apparently you
think they are not worth it, and while I'm surprised, I respect your
decision.
> > Side note #3: I'm surprised this change was needed in the first place
> > as my understanding was that gcc would skip all strict-aliasing
> > optimizations for void pointers, which is what the affected functions
> > are handling. It's sad that the commit message is as vague as gcc's
> > manual page on the topic.
>
> If a change makes some code more spec compliant, with little to no downside,
> I fail to see why it shouldn't be incldued.
>
> Your patch here makes code *less* spec compliant for little to no gain.
My code is doing the same that is already done everywhere in the ffmpeg
source code tree. It is (maybe) less spec compliant if you look only at
the function I modified. If you look at the project as a whole, it
doesn't make any difference. If such casts were already a problem, then
whoever is affected can already not use ffmpeg, with or without my
patch. Is it the case? Is there any platform where ffmpeg doesn't work
because of this? Any compiler that can't be used to build ffmpeg
because of this?
And this is why I'm surprised, really. There are only two positions
which I can understand. Either these casts are bad, they should be
removed from ffmpeg immediately, and no new occurrence should be
accepted. Or they are not bad, and there should be no objection
introducing new ones. Anything in the middle (as is happening right
now) makes no sense to me.
My own understanding of the situation is that such casts can be
problematic in certain cases [1] and this is why the compilers warn
about them. But the C spec allows for explicit cast between pointer
types for a reason, and that reason is that they are sometimes the
right thing to do. Which I think is the case here.
[1] http://c-faq.com/ansi/constmismatch.html
> P.S. It actually looks like the original code is not entirely compliant, after
> discussing with some people Smarter Than Me:
>
> [17:06] <+courmisch> memory copying the pointer fixes the aliasing problem on
> pointer itself
> [17:06] <+courmisch> but I suspect it leaves an aliasing problem with the
> pointed data
> [17:07] <+courmisch> specifically, I am not sure the standard allows creating
> pointers to existing data out of thin air
You do exactly that as soon as you pass a pointer as a parameter to a
functio
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
On Tue, 15 Dec 2015 16:56:02 +, Derek Buitenhuis wrote: > On 12/15/2015 10:44 AM, Jean Delvare wrote: > > Originally I proposed this patch for performance reasons and also > > because I think it makes the code more readable. But seeing how the > > same cast is already present everywhere in the ffmpeg code, I would now > > also invoke consistency. There's no rationale for not doing the same > > here that is already done everywhere else. If it caused any problem we > > would know by now (and I verified that this patch passes the test > > suite, FWIW.) > > I'd argue we should fix those instead. Some work on that has already been > done, > such as 60392480181f24ebf3ab48d8ac3614705de90152. Looks like something different from what we were discussing here. -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
On Tue, 15 Dec 2015 11:20:40 +0100, Michael Niedermayer wrote: > yes, I have no real oppinion on this except that C is insane or I am C is insane, who would dare to argue otherwise? ;-) More than the language itself, the fact that the compilers think they can reorder instructions the way they like has always frightened me. I prefer when tools obey to whoever if handling them, rather than making their own decisions. -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
Hi Hendrik, On Tue, 15 Dec 2015 11:31:57 +0100, Hendrik Leppkes wrote: > On Tue, Dec 15, 2015 at 11:20 AM, Michael Niedermayer <michae...@gmx.at> > wrote: > > On Tue, Dec 15, 2015 at 08:58:01AM +0100, Jean Delvare wrote: > >> Also I can see 12 occurrences of the same cast for this parameter of > >> function av_image_copy() in the ffmpeg code already. And over 20 more > >> similar casts for similar parameters of other functions > >> (ff_combine_frame, swr_convert, copy_picture_field...) So I'm not > >> introducing anything new, just proposing one more of the same. > > > > yes, I have no real oppinion on this except that C is insane or I am > > and i dont really mind to apply the patch if thats what people prefer. > > Any real compiler that follows this litterally and breaks the code is > > IMHO a compiler one should avoid (quite independant of it being used > > for FFmpeg or other things) unless one wants to language lawyer around > > on such things instead of writing usefull code > > The "speed up" from removing a copy of 4 pointers is negligible as > well however, so maybe it should just be kept like it is. Originally I proposed this patch for performance reasons and also because I think it makes the code more readable. But seeing how the same cast is already present everywhere in the ffmpeg code, I would now also invoke consistency. There's no rationale for not doing the same here that is already done everywhere else. If it caused any problem we would know by now (and I verified that this patch passes the test suite, FWIW.) -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
Hi Derek,
On Tue, 15 Dec 2015 17:39:23 +, Derek Buitenhuis wrote:
> On 12/15/2015 5:23 PM, Jean Delvare wrote:
> > Looks like something different from what we were discussing here.
>
> In which way?
>
> That patch fixes pointer aliasing in the same way yours breaks it, AFAICT?
I see two completely different things.
The change I proposed is specific to one function, only changes const
vs non-const, and the object under discussion is being accessed for
reading only (thus the const pointer.) It would remove a memcpy but
does not introduce a direct copy (with =.)
The change you pointed us to is touching very generic functions,
handling by nature a very wide variety of pointer types for both
reading and writing.
So just because the change you pointed us to may be good and desirable,
doesn't imply that the change I am proposing is bad and undesirable.
Side note #1: if gcc really considers pointer types that only differ by
"const" as different when it comes to code optimization, it is
seriously broken IMHO. Unfortunately the man page is quite vague on the
topic ("unless the types are almost the same", can you be more vague
please.)
Side note #2: if strict-aliasing optimizations can do so much damage
to your code that you end up doing
memcpy(arg, &(void *){ NULL }, sizeof(val));
to set a pointer to NULL, maybe you should consider building with
-fno-strict-aliasing.
Side note #3: I'm surprised this change was needed in the first place
as my understanding was that gcc would skip all strict-aliasing
optimizations for void pointers, which is what the affected functions
are handling. It's sad that the commit message is as vague as gcc's
manual page on the topic.
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/vf_delogo: fix show option when clipping
The show option did not take clipping into account, so the borders on
the clipped side wouldn't show up. Fix it.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
libavfilter/vf_delogo.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-12-11 14:59:30.539475316 +0100
+++ ffmpeg/libavfilter/vf_delogo.c 2015-12-11 14:59:31.264491882 +0100
@@ -101,8 +101,8 @@ static void apply_delogo(uint8_t *dst, i
xdst = dst+logo_x1+1,
xsrc = src+logo_x1+1; x < logo_x2-1; x++, xdst++, xsrc++) {
-if (show && (y == logo_y+1 || y == logo_y+logo_h-2 ||
- x == logo_x+1 || x == logo_x+logo_w-2)) {
+if (show && (y == logo_y1+1 || y == logo_y2-2 ||
+ x == logo_x1+1 || x == logo_x2-2)) {
*xdst = 0;
continue;
}
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
Hallo Michael,
On Mon, 14 Dec 2015 23:18:39 +0100, Michael Niedermayer wrote:
> On Mon, Dec 14, 2015 at 07:36:51PM +0100, Jean Delvare wrote:
> > As I understand it, the temporary stack buffer "src_data" was
> > introduced solely to avoid a compiler warning. I believe that a better
> > way to solve this warning it to explicitly cast src->data. This
> > should be somewhat faster, and just as safe.
> >
> > Signed-off-by: Jean Delvare <jdelv...@suse.de>
> > Cc: Anton Khirnov <an...@khirnov.net>
> > ---
> > libavutil/frame.c |4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > --- ffmpeg.orig/libavutil/frame.c 2015-12-14 18:42:06.272234579 +0100
> > +++ ffmpeg/libavutil/frame.c2015-12-14 19:05:18.501745387 +0100
> > @@ -647,7 +647,6 @@ AVFrameSideData *av_frame_get_side_data(
> >
> > static int frame_copy_video(AVFrame *dst, const AVFrame *src)
> > {
> > -const uint8_t *src_data[4];
> > int i, planes;
> >
> > if (dst->width < src->width ||
> > @@ -659,9 +658,8 @@ static int frame_copy_video(AVFrame *dst
> > if (!dst->data[i] || !src->data[i])
> > return AVERROR(EINVAL);
> >
> > -memcpy(src_data, src->data, sizeof(src_data));
> > av_image_copy(dst->data, dst->linesize,
> > - src_data, src->linesize,
> > + (const uint8_t **)src->data, src->linesize,
>
> I think this may be a aliasing violation and thus undefined
> not that i like that or consider that sane
> so if someone says iam wrong, i would certainly not be unhappy
Why would it be an aliasing violation? We do not change the fundamental
type of the pointer, we only add a const where the function wants it.
For more simple pointer types the compiler would do it for us silently.
Also I can see 12 occurrences of the same cast for this parameter of
function av_image_copy() in the ffmpeg code already. And over 20 more
similar casts for similar parameters of other functions
(ff_combine_frame, swr_convert, copy_picture_field...) So I'm not
introducing anything new, just proposing one more of the same.
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavu/frame: Optimize frame_copy_video
As I understand it, the temporary stack buffer "src_data" was
introduced solely to avoid a compiler warning. I believe that a better
way to solve this warning it to explicitly cast src->data. This
should be somewhat faster, and just as safe.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
Cc: Anton Khirnov <an...@khirnov.net>
---
libavutil/frame.c |4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
--- ffmpeg.orig/libavutil/frame.c 2015-12-14 18:42:06.272234579 +0100
+++ ffmpeg/libavutil/frame.c2015-12-14 19:05:18.501745387 +0100
@@ -647,7 +647,6 @@ AVFrameSideData *av_frame_get_side_data(
static int frame_copy_video(AVFrame *dst, const AVFrame *src)
{
-const uint8_t *src_data[4];
int i, planes;
if (dst->width < src->width ||
@@ -659,9 +658,8 @@ static int frame_copy_video(AVFrame *dst
if (!dst->data[i] || !src->data[i])
return AVERROR(EINVAL);
-memcpy(src_data, src->data, sizeof(src_data));
av_image_copy(dst->data, dst->linesize,
- src_data, src->linesize,
+ (const uint8_t **)src->data, src->linesize,
dst->format, src->width, src->height);
return 0;
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Question about av_frame_is_writable
Hi all, Can av_frame_is_writable() ever return 1, and if so, when? Context: I am testing corner cases of the delogo filter. To my surprise, even for a simple test case such as: $ ffmpeg -f lavfi -i "color=color=white:size=24x24" -frames:v 1 -vf "delogo=7:3:8:8" -f image2 debug.png av_frame_is_writable() returns 0, which disables direct mode of the delogo filter and forces the allocation of a new buffer. My understanding was that we were trying to avoid allocation and data copy as much as possible, for performance reasons, and with such a simple filter graph I can't see why we can't modify the video data in-place. Can someone enlighten me? Thanks, -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Question about av_frame_is_writable
Hi Paul, On Sat, 12 Dec 2015 12:03:39 +, Paul B Mahol wrote: > On 12/12/15, Jean Delvare <jdelv...@suse.de> wrote: > > Can av_frame_is_writable() ever return 1, and if so, when? > > > > Context: I am testing corner cases of the delogo filter. To my > > surprise, even for a simple test case such as: > > > > $ ffmpeg -f lavfi -i "color=color=white:size=24x24" -frames:v 1 -vf > > "delogo=7:3:8:8" -f image2 debug.png > > > > av_frame_is_writable() returns 0, which disables direct mode of the > > delogo filter and forces the allocation of a new buffer. My > > understanding was that we were trying to avoid allocation and data copy > > as much as possible, for performance reasons, and with such a simple > > filter graph I can't see why we can't modify the video data in-place. > > Can someone enlighten me? > > Non-refcounted frames are never writable. And color source is made of one > single > frame which is cloned multiple times. > Use different input file perhaps? Thanks for the hint. That makes sense, and if I set the input to a still picture instead then av_frame_is_writable() indeed returns 1. However if I add replace "-frames:v 1" with "-loop 1 -frames:v 5", av_frame_is_writable() still returns 1. I expected the input picture to be cloned 4 times so it would no longer be writable, given your explanation above. Also if I change the input to a MPEG file, av_frame_is_writable() returns 0 again. So I still do not fully understand when frames are writable and when not. But -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Detecting invalid filter parameters
On Thu, 10 Dec 2015 08:58:05 +0100, Nicolas George wrote: > Le decadi 20 frimaire, an CCXXIV, Jean Delvare a écrit : > > This brings two additional questions: > > > > 1* Can a filter declare which midstream changes it does or does not > > support? > > > > 2* Can a filter be called back on midstream changes, or does it have to > > detect it by itself? > > Midstream changes are not supported yet at all. They work by chance for a > few selected whitelisted filters, that is all. No need to worry yet. OK, thanks. That will make things easier. I'll write down a note before the checks on what would need to be done if support is ever added. -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH RFC] avfilter/vf_delogo: Use AVPixFmtDescriptor.nb_components
Relying on AVPixFmtDescriptor.nb_components is cleaner and faster than
checking data and linesize for every possible plane.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
I see that most filters use AVPixFmtDescriptor.nb_components while
others still check data and linesize. Am I correct assuming that the
former is faster and preferred?
libavfilter/vf_delogo.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-12-10 13:06:16.212908502 +0100
+++ ffmpeg/libavfilter/vf_delogo.c 2015-12-10 13:07:04.877966120 +0100
@@ -256,7 +256,7 @@ static int filter_frame(AVFilterLink *in
if (!sar.num)
sar.num = sar.den = 1;
-for (plane = 0; plane < 4 && in->data[plane] && in->linesize[plane];
plane++) {
+for (plane = 0; plane < desc->nb_components; plane++) {
int hsub = plane == 1 || plane == 2 ? hsub0 : 0;
int vsub = plane == 1 || plane == 2 ? vsub0 : 0;
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/vf_delogo: round to the closest value
When the interpolated value is divided by the sum of weights, no
rounding is done, which means the value is truncated. This results in
a slight bias towards dark green in the interpolated area. Rounding
properly removes the bias.
I measured this change to reduce the interpolation error by 1 to 2 %
on average on a number of sample input and logo area combinations.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
We are having hack week 13 at SUSE and my project this time is to
improve and clean up vf_delogo.
Note: All testing was done with band=1 as this is the new default
value. I purposely left the code branch dealing with the band area
untouched as it will ultimately be removed anyway.
libavfilter/vf_delogo.c |5
tests/ref/fate/filter-delogo | 218 +--
2 files changed, 112 insertions(+), 111 deletions(-)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-12-08 08:21:57.291549315 +0100
+++ ffmpeg/libavfilter/vf_delogo.c 2015-12-08 20:58:04.859399469 +0100
@@ -61,7 +61,7 @@ static void apply_delogo(uint8_t *dst, i
unsigned int band, int show, int direct)
{
int x, y;
-uint64_t interp, weightl, weightr, weightt, weightb;
+uint64_t interp, weightl, weightr, weightt, weightb, weight;
uint8_t *xdst, *xsrc;
uint8_t *topleft, *botleft, *topright;
@@ -125,7 +125,8 @@ static void apply_delogo(uint8_t *dst, i
(botleft[x-logo_x1]+
botleft[x-logo_x1-1] +
botleft[x-logo_x1+1]) * weightb;
-interp /= (weightl + weightr + weightt + weightb) * 3U;
+weight = (weightl + weightr + weightt + weightb) * 3U;
+interp = ROUNDED_DIV(interp, weight);
if (y >= logo_y+band && y < logo_y+logo_h-band &&
x >= logo_x+band && x < logo_x+logo_w-band) {
--- ffmpeg.orig/tests/ref/fate/filter-delogo2015-12-08 23:06:51.819541763
+0100
+++ ffmpeg/tests/ref/fate/filter-delogo 2015-12-08 23:07:01.262743627 +0100
@@ -1,110 +1,110 @@
#tb 0: 32768/982057
-0, 0, 0,1, 126720, 0xd975ec13
-0, 1, 1,1, 126720, 0xae91ecb1
-0, 2, 2,1, 126720, 0xae91ecb1
-0, 3, 3,1, 126720, 0xae91ecb1
-0, 4, 4,1, 126720, 0x6b51ecf3
-0, 5, 5,1, 126720, 0x3015f463
-0, 6, 6,1, 126720, 0x68eaf4a3
-0, 7, 7,1, 126720, 0xd68bf483
-0, 8, 8,1, 126720, 0xa8c0b7e3
-0, 9, 9,1, 126720, 0x1bf4b8a3
-0, 10, 10,1, 126720, 0x1546b8e3
-0, 11, 11,1, 126720, 0x9ac0b8c7
-0, 12, 12,1, 126720, 0x7de8b913
-0, 13, 13,1, 126720, 0xfd78bb83
-0, 14, 14,1, 126720, 0x5bd9bc03
-0, 15, 15,1, 126720, 0xa6eebba7
-0, 16, 16,1, 126720, 0x42e4b8f3
-0, 17, 17,1, 126720, 0xd97fadf0
-0, 18, 18,1, 126720, 0xf28299ce
-0, 19, 19,1, 126720, 0x9843a809
-0, 20, 20,1, 126720, 0x619aba40
-0, 21, 21,1, 126720, 0xe216a860
-0, 22, 22,1, 126720, 0xe2ccab69
-0, 23, 23,1, 126720, 0x4e2caa85
-0, 24, 24,1, 126720, 0x11c9bca0
-0, 25, 25,1, 126720, 0xc13da72d
-0, 26, 26,1, 126720, 0x894fed60
-0, 27, 27,1, 126720, 0xa3f0b765
-0, 28, 28,1, 126720, 0x645d06eb
-0, 29, 29,1, 126720, 0xfcfd88a8
-0, 30, 30,1, 126720, 0xe73704a2
-0, 31, 31,1, 126720, 0xa548bdf5
-0, 32, 32,1, 126720, 0x2b0207b7
-0, 33, 33,1, 126720, 0x8fd6d84c
-0, 34, 34,1, 126720, 0x1c1fde83
-0, 35, 35,1, 126720, 0x1b69afd3
-0, 36, 36,1, 126720, 0x8c487b48
-0, 37, 37,1, 126720, 0x0e0fb90a
-0, 38, 38,1, 126720, 0x0b6ba745
-0, 39, 39,1, 126720, 0xfe09d22e
-0, 40, 40,1, 126720, 0x686bff72
-0, 41, 41,1, 126720, 0x5b7e4f75
-0, 42, 42,1, 126720, 0x8fa61ee2
-0, 43, 43,1, 126720, 0xa26462ef
-0, 44, 44,1, 126720, 0x362d6606
-0, 45, 45,1, 126720, 0x53faca36
-0, 46, 46,1, 126720, 0xc0cacf66
-0, 47, 47,1, 126720, 0xd3cbe8d2
-0, 48, 48,
[FFmpeg-devel] Detecting invalid filter parameters
Hi FFmpeg developers, The delogo video filter currently doesn't check the logo area passed as parameters for validity. If the logo area is partly outside of the frame or inside but too close to the border, the code will silently trim the area to make it fit inside the frame, then go on, with undesirable results. I would like to add a check for the delogo filter parameters to ensure that there is enough frame room around the area to execute the algorithm. My question is: do I have to do this check in filter_frame() for every frame, or can I do it once in init() and be done with it? Or is it supposed to be done in config_props()? I read the documentation about config_props but I'm afraid it did not really enlightened me as to what this callback is for. For performance reasons I'd like to avoid performing the same check repeatedly. Sorry if this sounds like a newbie question but well I guess that's what I am. I'm still not sure what is the difference between filter links and pads (or more generally what pads are)... Thanks, -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Detecting invalid filter parameters
Hi Paul, Thanks for your quick reply. On Wed, 9 Dec 2015 19:45:16 +, Paul B Mahol wrote: > On 12/9/15, Jean Delvare <jdelv...@suse.de> wrote: > > I would like to add a check for the delogo filter parameters to ensure > > that there is enough frame room around the area to execute the > > algorithm. > > > > My question is: do I have to do this check in filter_frame() for every > > frame, or can I do it once in init() and be done with it? Or is it > > supposed to be done in config_props()? I read the documentation about > > config_props but I'm afraid it did not really enlightened me as to what > > this callback is for. For performance reasons I'd like to avoid > > performing the same check repeatedly. > > If filter doesn't support midstream resolution changes than its ok > to do it in config_props for one of input or output pads. OK. I found that I couldn't do it in init() because ctx->inputs[0] is NULL at that point. So I implemented .config_props. Midstream resolution change would make no sense for the delogo filter. Pixel format changes would be supportable though and the check would have to be done again due to possibly different subsampling factors. This brings two additional questions: 1* Can a filter declare which midstream changes it does or does not support? 2* Can a filter be called back on midstream changes, or does it have to detect it by itself? Thanks, -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avfilter/delogo: Set default band to 1
On Thu, 8 Oct 2015 12:20:25 +0200, Hendrik Leppkes wrote: > On Thu, Oct 8, 2015 at 11:30 AM, Stefano Sabatini <stefa...@gmail.com> wrote: > > On date Wednesday 2015-10-07 15:03:32 +0200, Jean Delvare encoded: > >> The original interpolation algorithm behaved poorly on the borders and > >> did not even guarantee continuity at the borders. For this reason, a > >> second interpolation/blending pass was required on the borders to make > >> them seamless. > >> > >> However, since the interpolation algorithm was improved in June 2013, > >> the border issues no longer exist. The new algorithm does guarantee > >> continuity at the borders, making the second pass useless. A larger > >> band always increases the cumulated interpolation error. In most cases > >> it also increases the average interpolation error, even though the > >> samples in the band are only partially interpolated. > >> > >> For this reason I would like to get rid of the "band" parameter. As a > >> first step, let's change its default value from 4 to 1 and document it > >> as deprecated. > >> > >> I have benchmarked this change on a combination of input sources and > >> realistic logo areas. Lowering the band value from 4 to 1 resulted in > >> 8 to 39 % less interpolation error per frame (or 1 to 34 % less > >> interpolation error per luma sample.) > >> > >> Signed-off-by: Jean Delvare <jdelv...@suse.de> > >> --- > >> Changes since v1: > >> * Added #ifs so that the deprecated options are dropped automatically > >>on next major version of libavfilter (suggested by Stefano Sabatini) > >> > >> doc/filters.texi|4 +++- > >> libavfilter/vf_delogo.c | 17 +++-- > >> 2 files changed, 18 insertions(+), 3 deletions(-) > > > > Thanks, applied. > > This seems to have broken FATE, ie: > http://fate.ffmpeg.org/report.cgi?time=20151008101706=x86_32-mingw-w64-dll-windows-native Oops, yes, sorry. The different result is intended. I meant to update the test result but forgot to do so. Patch coming. -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avfilter/delogo: Set default band to 1
On Thu, 8 Oct 2015 13:00:33 +0200, Stefano Sabatini wrote: > On date Thursday 2015-10-08 12:42:34 +0200, Jean Delvare encoded: > > On Thu, 8 Oct 2015 12:20:25 +0200, Hendrik Leppkes wrote: > [...] > > > This seems to have broken FATE, ie: > > > http://fate.ffmpeg.org/report.cgi?time=20151008101706=x86_32-mingw-w64-dll-windows-native > > > > Oops, yes, sorry. The different result is intended. I meant to update > > the test result but forgot to do so. Patch coming. > > It was my fault as committer. It should be already fixed on master. You are too nice with me Stefano. Thank you for quickly fixing my mistake! -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/delogo: Set default band to 1
The original interpolation algorithm behaved poorly on the borders and
did not even guarantee continuity at the borders. For this reason, a
second interpolation/blending pass was required on the borders to make
them seamless.
However, since the interpolation algorithm was improved in June 2013,
the border issues no longer exist. The new algorithm does guarantee
continuity at the borders, making the second pass useless. A larger
band always increases the cumulated interpolation error. In most cases
it also increases the average interpolation error, even though the
samples in the band are only partially interpolated.
For this reason I would like to get rid of the "band" parameter. As a
first step, let's change its default value from 4 to 1 and document it
as deprecated.
I have benchmarked this change on a combination of input sources and
realistic logo areas. Lowering the band value from 4 to 1 resulted in
8 to 39 % less interpolation error per frame (or 1 to 34 % less
interpolation error per luma sample.)
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
doc/filters.texi|4 +++-
libavfilter/vf_delogo.c | 11 +--
2 files changed, 12 insertions(+), 3 deletions(-)
--- ffmpeg.orig/doc/filters.texi2015-10-02 11:41:24.035943770 +0200
+++ ffmpeg/doc/filters.texi 2015-10-02 12:02:08.231484828 +0200
@@ -4590,7 +4590,9 @@ specified.
@item band, t
Specify the thickness of the fuzzy edge of the rectangle (added to
-@var{w} and @var{h}). The default value is 4.
+@var{w} and @var{h}). The default value is 1. This option is
+deprecated, setting higher values should no longer be necessary and
+is not recommended.
@item show
When set to 1, a green rectangle is drawn on the screen to simplify
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-10-02 11:41:24.035943770 +0200
+++ ffmpeg/libavfilter/vf_delogo.c 2015-10-07 08:45:39.037203764 +0200
@@ -165,8 +165,9 @@ static const AVOption delogo_options[]=
{ "y","set logo y position", OFFSET(y),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
{ "w","set logo width",OFFSET(w),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
{ "h","set logo height", OFFSET(h),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
-{ "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, {
.i64 = 4 }, 1, INT_MAX, FLAGS },
-{ "t","set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, {
.i64 = 4 }, 1, INT_MAX, FLAGS },
+/* Actual default value for band/t is 1, set in init */
+{ "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 },
{ "show", "show delogo area", OFFSET(show), AV_OPT_TYPE_BOOL,{
.i64 = 0 }, 0, 1, FLAGS },
{ NULL }
};
@@ -201,6 +202,12 @@ static av_cold int init(AVFilterContext
CHECK_UNSET_OPT(w);
CHECK_UNSET_OPT(h);
+if (s->band == 0) { /* Unset, use default */
+av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed from
4 to 1.\n");
+s->band = 1;
+} else if (s->band != 1) {
+av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
+}
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);
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/delogo: Set default band to 1
Hi Stefano,
On Wed, 7 Oct 2015 11:21:45 +0200, Stefano Sabatini wrote:
> On date Wednesday 2015-10-07 08:54:59 +0200, Jean Delvare encoded:
> > The original interpolation algorithm behaved poorly on the borders and
> > did not even guarantee continuity at the borders. For this reason, a
> > second interpolation/blending pass was required on the borders to make
> > them seamless.
> >
> > However, since the interpolation algorithm was improved in June 2013,
> > the border issues no longer exist. The new algorithm does guarantee
> > continuity at the borders, making the second pass useless. A larger
> > band always increases the cumulated interpolation error. In most cases
> > it also increases the average interpolation error, even though the
> > samples in the band are only partially interpolated.
> >
> > For this reason I would like to get rid of the "band" parameter. As a
> > first step, let's change its default value from 4 to 1 and document it
> > as deprecated.
> >
> > I have benchmarked this change on a combination of input sources and
> > realistic logo areas. Lowering the band value from 4 to 1 resulted in
> > 8 to 39 % less interpolation error per frame (or 1 to 34 % less
> > interpolation error per luma sample.)
> >
> > Signed-off-by: Jean Delvare <jdelv...@suse.de>
> > ---
> > doc/filters.texi|4 +++-
> > libavfilter/vf_delogo.c | 11 +--
> > 2 files changed, 12 insertions(+), 3 deletions(-)
> >
> > --- ffmpeg.orig/doc/filters.texi2015-10-02 11:41:24.035943770 +0200
> > +++ ffmpeg/doc/filters.texi 2015-10-02 12:02:08.231484828 +0200
> > @@ -4590,7 +4590,9 @@ specified.
> >
> > @item band, t
> > Specify the thickness of the fuzzy edge of the rectangle (added to
> > -@var{w} and @var{h}). The default value is 4.
> > +@var{w} and @var{h}). The default value is 1. This option is
> > +deprecated, setting higher values should no longer be necessary and
> > +is not recommended.
> >
> > @item show
> > When set to 1, a green rectangle is drawn on the screen to simplify
> > --- ffmpeg.orig/libavfilter/vf_delogo.c 2015-10-02 11:41:24.035943770
> > +0200
> > +++ ffmpeg/libavfilter/vf_delogo.c 2015-10-07 08:45:39.037203764 +0200
> > @@ -165,8 +165,9 @@ static const AVOption delogo_options[]=
> > { "y","set logo y position", OFFSET(y),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > { "w","set logo width",OFFSET(w),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > { "h","set logo height", OFFSET(h),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > -{ "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT,
> > { .i64 = 4 }, 1, INT_MAX, FLAGS },
> > -{ "t","set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT,
> > { .i64 = 4 }, 1, INT_MAX, FLAGS },
> > +/* Actual default value for band/t is 1, set in init */
> > +{ "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 },
> > { "show", "show delogo area", OFFSET(show),
> > AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, FLAGS },
> > { NULL }
> > };
> > @@ -201,6 +202,12 @@ static av_cold int init(AVFilterContext
> > CHECK_UNSET_OPT(w);
> > CHECK_UNSET_OPT(h);
> >
> > +if (s->band == 0) { /* Unset, use default */
> > +av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed
> > from 4 to 1.\n");
> > +s->band = 1;
> > +} else if (s->band != 1) {
> > +av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
> > +}
> > 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);
>
> LGTM. BTW, if you want to drop the band option, you could ifdef it so
> that it will be dropt at the next lavfi major bump.
Good idea. Something like this?
---
libavfilter/vf_delogo.c |6 ++
1 file changed, 6 insertions(+)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-10-07 08:45:39.037203764 +0200
+++ ffmpeg/libavfilter/vf_delogo.c 2015-10-07 13:52:43.716487733 +0200
@
Re: [FFmpeg-devel] [PATCH] avfilter/delogo: Set default band to 1
On Wed, 7 Oct 2015 14:19:58 +0200, Stefano Sabatini wrote:
> > > LGTM. BTW, if you want to drop the band option, you could ifdef it so
> > > that it will be dropt at the next lavfi major bump.
> >
> > Good idea. Something like this?
> >
> > ---
> > libavfilter/vf_delogo.c |6 ++
> > 1 file changed, 6 insertions(+)
> >
> > --- ffmpeg.orig/libavfilter/vf_delogo.c 2015-10-07 08:45:39.037203764
> > +0200
> > +++ ffmpeg/libavfilter/vf_delogo.c 2015-10-07 13:52:43.716487733 +0200
> > @@ -165,9 +165,11 @@ static const AVOption delogo_options[]=
> > { "y","set logo y position", OFFSET(y),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > { "w","set logo width",OFFSET(w),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > { "h","set logo height", OFFSET(h),AV_OPT_TYPE_INT,
> > { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > +#if LIBAVFILTER_VERSION_MAJOR < 7
> > /* Actual default value for band/t is 1, set in init */
> > { "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
> > { "show", "show delogo area", OFFSET(show),
> > AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, FLAGS },
> > { NULL }
> > };
> > @@ -202,12 +204,16 @@ static av_cold int init(AVFilterContext
> > CHECK_UNSET_OPT(w);
> > CHECK_UNSET_OPT(h);
> >
> > +#if LIBAVFILTER_VERSION_MAJOR < 7
> > if (s->band == 0) { /* Unset, use default */
> > av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed
> > from 4 to 1.\n");
> > s->band = 1;
> > } else if (s->band != 1) {
> > av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
> > }
> > +#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);
>
> Looks fine to me.
Great.
> > Or should I use FF_API_OLD_FILTER_OPTS?
>
> No, I think that's related to another issue.
I thought so but wasn't sure.
> > Assuming this is what you had in mind, would it go as a separate patch,
> > or should this be folded in the patch I just sent?
>
> Use a single patch since it involves less work, unless you prefer to
> do it with a separate patch.
I'm fine either way so if a single patch is easier for you, I'll do
that. I'll send a v2 of the patch in a minute.
Thanks for the review,
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v2] avfilter/delogo: Set default band to 1
The original interpolation algorithm behaved poorly on the borders and
did not even guarantee continuity at the borders. For this reason, a
second interpolation/blending pass was required on the borders to make
them seamless.
However, since the interpolation algorithm was improved in June 2013,
the border issues no longer exist. The new algorithm does guarantee
continuity at the borders, making the second pass useless. A larger
band always increases the cumulated interpolation error. In most cases
it also increases the average interpolation error, even though the
samples in the band are only partially interpolated.
For this reason I would like to get rid of the "band" parameter. As a
first step, let's change its default value from 4 to 1 and document it
as deprecated.
I have benchmarked this change on a combination of input sources and
realistic logo areas. Lowering the band value from 4 to 1 resulted in
8 to 39 % less interpolation error per frame (or 1 to 34 % less
interpolation error per luma sample.)
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
Changes since v1:
* Added #ifs so that the deprecated options are dropped automatically
on next major version of libavfilter (suggested by Stefano Sabatini)
doc/filters.texi|4 +++-
libavfilter/vf_delogo.c | 17 +++--
2 files changed, 18 insertions(+), 3 deletions(-)
--- ffmpeg.orig/doc/filters.texi2015-10-02 11:41:24.035943770 +0200
+++ ffmpeg/doc/filters.texi 2015-10-02 12:02:08.231484828 +0200
@@ -4590,7 +4590,9 @@ specified.
@item band, t
Specify the thickness of the fuzzy edge of the rectangle (added to
-@var{w} and @var{h}). The default value is 4.
+@var{w} and @var{h}). The default value is 1. This option is
+deprecated, setting higher values should no longer be necessary and
+is not recommended.
@item show
When set to 1, a green rectangle is drawn on the screen to simplify
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-10-02 11:41:24.035943770 +0200
+++ ffmpeg/libavfilter/vf_delogo.c 2015-10-07 14:56:43.521386501 +0200
@@ -165,8 +165,11 @@ static const AVOption delogo_options[]=
{ "y","set logo y position", OFFSET(y),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
{ "w","set logo width",OFFSET(w),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
{ "h","set logo height", OFFSET(h),AV_OPT_TYPE_INT, {
.i64 = -1 }, -1, INT_MAX, FLAGS },
-{ "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, {
.i64 = 4 }, 1, INT_MAX, FLAGS },
-{ "t","set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, {
.i64 = 4 }, 1, INT_MAX, FLAGS },
+#if LIBAVFILTER_VERSION_MAJOR < 7
+/* Actual default value for band/t is 1, set in init */
+{ "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
{ "show", "show delogo area", OFFSET(show), AV_OPT_TYPE_BOOL,{
.i64 = 0 }, 0, 1, FLAGS },
{ NULL }
};
@@ -201,6 +204,16 @@ static av_cold int init(AVFilterContext
CHECK_UNSET_OPT(w);
CHECK_UNSET_OPT(h);
+#if LIBAVFILTER_VERSION_MAJOR < 7
+if (s->band == 0) { /* Unset, use default */
+av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed from
4 to 1.\n");
+s->band = 1;
+} else if (s->band != 1) {
+av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
+}
+#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);
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/delogo: Fix show option when band is small
The code assumed that the outermost interpolated pixels were always in
the fuzzy area defined by the band option. However if the band value
is small, there may be no fuzzy area on a given plane. In that case,
option show did not work, no rectangle was drawn (or only on the luma
plane, depending on the band value and chroma plane subsampling
factors.)
Fix the problem by not making any assumption on where the outermost
interpolated pixels will be.
The new code was verified to produce the same result as the original
code when the band value is not small.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
libavfilter/vf_delogo.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
--- ffmpeg.orig/libavfilter/vf_delogo.c 2015-09-28 08:31:19.151967866 +0200
+++ ffmpeg/libavfilter/vf_delogo.c 2015-09-28 10:03:32.733287345 +0200
@@ -1,7 +1,7 @@
/*
* Copyright (c) 2002 Jindrich Makovicka <makov...@gmail.com>
* Copyright (c) 2011 Stefano Sabatini
- * Copyright (c) 2013 Jean Delvare <jdelv...@suse.com>
+ * Copyright (c) 2013, 2015 Jean Delvare <jdelv...@suse.com>
*
* This file is part of FFmpeg.
*
@@ -101,6 +101,12 @@ static void apply_delogo(uint8_t *dst, i
xdst = dst+logo_x1+1,
xsrc = src+logo_x1+1; x < logo_x2-1; x++, xdst++, xsrc++) {
+if (show && (y == logo_y+1 || y == logo_y+logo_h-2 ||
+ x == logo_x+1 || x == logo_x+logo_w-2)) {
+*xdst = 0;
+continue;
+}
+
/* Weighted interpolation based on relative distances, taking SAR
into account */
weightl = (uint64_t) (logo_x2-1-x) * (y-logo_y1) *
(logo_y2-1-y) * sar.den;
weightr = (uint64_t)(x-logo_x1) * (y-logo_y1) *
(logo_y2-1-y) * sar.den;
@@ -138,8 +144,6 @@ static void apply_delogo(uint8_t *dst, i
dist = FFMAX(dist, y-(logo_y+logo_h-1-band));
*xdst = (*xsrc*dist + interp*(band-dist))/band;
-if (show && (dist == band-1))
-*xdst = 0;
}
}
--
Jean Delvare
SUSE L3 Support
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Update my email address
My old address no longer works. --- MAINTAINERS |2 +- libavfilter/vf_delogo.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) --- ffmpeg.orig/MAINTAINERS 2015-09-16 08:51:37.861444714 +0200 +++ ffmpeg/MAINTAINERS 2015-09-22 10:38:01.760054763 +0200 @@ -365,7 +365,7 @@ Filters: vf_colorlevels.c Paul B Mahol vf_deband.c Paul B Mahol vf_dejudder.c Nicholas Robbins - vf_delogo.c Jean Delvare (CC <kh...@linux-fr.org>) + vf_delogo.c Jean Delvare (CC <jdelv...@suse.com>) vf_drawbox.c/drawgrid Andrey Utkin vf_extractplanes.cPaul B Mahol vf_histogram.cPaul B Mahol --- ffmpeg.orig/libavfilter/vf_delogo.c 2015-09-22 10:37:18.779192905 +0200 +++ ffmpeg/libavfilter/vf_delogo.c 2015-09-22 10:37:51.412847063 +0200 @@ -1,7 +1,7 @@ /* * Copyright (c) 2002 Jindrich Makovicka <makov...@gmail.com> * Copyright (c) 2011 Stefano Sabatini - * Copyright (c) 2013 Jean Delvare <kh...@linux-fr.org> + * Copyright (c) 2013 Jean Delvare <jdelv...@suse.com> * * This file is part of FFmpeg. * -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] cmdutils: fix success path
Since commit 934f2d2f5c16df8aad9f401a9fd842b5d9a78b11, cmdutils_read_file() prints a confusing message on success: IO error: Success This is because the error message is printed on the success path as well. Add the missing condition so that it is only printed on error. Signed-off-by: Jean Delvare jdelv...@suse.de Cc: Michael Niedermayer michae...@gmx.at --- cmdutils.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- ffmpeg.orig/cmdutils.c 2015-02-11 14:01:08.995929450 +0100 +++ ffmpeg/cmdutils.c 2015-02-11 14:02:30.827593418 +0100 @@ -1912,7 +1912,8 @@ int cmdutils_read_file(const char *filen } out: -av_log(NULL, AV_LOG_ERROR, IO error: %s\n, av_err2str(ret)); +if (ret 0) + av_log(NULL, AV_LOG_ERROR, IO error: %s\n, av_err2str(ret)); fclose(f); return ret; } -- Jean Delvare SUSE L3 Support ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
