diff mbox

[FFmpeg-devel] avfilter/drawtext - implement fix_bounds

Message ID 55b6405d-c947-0f78-ddaf-4dcc8d67a13c@gmail.com
State Accepted
Commit 6c1c6c6c71fc776c6dd25d13861b036dad2cdc1b
Headers show

Commit Message

Gyan Jan. 11, 2018, 1:42 p.m. UTC
The no-op commit e496c45 from Libav introduced an option which allowed 
the user to relocate text to fit within the frame if it was going out of 
bounds.

For some reason, when the merge commit b479e01 was applied, the option 
was added but the code fragment which implemented it, was not. So the 
option was dead on arrival, and has remained impotent (since Feb 2012).

Attached patch implements option. Text styling elements like shadow or 
box are also kept within bounds. Default value changed to false so that 
filter outcome doesn't change in existing scripts.

Regards,
Gyan
From d74b718d75db0ab6724ca807e2af1c1e756f7c56 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <gyandoshi@gmail.com>
Date: Thu, 11 Jan 2018 18:30:16 +0530
Subject: [PATCH] avfilter/drawtext - implement fix_bounds

When enabled, text, including effects like shadow or box, will be
completely bound within the video frame.

Default value changed to false to keep continuity of behaviour.

Fixes #6960.
---
 libavfilter/vf_drawtext.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Kyle Swanson Jan. 12, 2018, 8:31 a.m. UTC | #1
Hi,

On Thu, Jan 11, 2018 at 5:42 AM, Gyan Doshi <gyandoshi@gmail.com> wrote:

> The no-op commit e496c45 from Libav introduced an option which allowed the
> user to relocate text to fit within the frame if it was going out of bounds.
>
> For some reason, when the merge commit b479e01 was applied, the option was
> added but the code fragment which implemented it, was not. So the option
> was dead on arrival, and has remained impotent (since Feb 2012).
>
> Attached patch implements option. Text styling elements like shadow or box
> are also kept within bounds. Default value changed to false so that filter
> outcome doesn't change in existing scripts.
>
> Regards,
> Gyan
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Just read through the patch, but actually didn't try it yet. I wonder if we
should also scale the size of the text in cases where it's wider than the
frame?

Thanks,
Kyle
Gyan Jan. 12, 2018, 1:42 p.m. UTC | #2
On 1/12/2018 2:01 PM, Kyle Swanson wrote:

> Just read through the patch, but actually didn't try it yet. I wonder if we
> should also scale the size of the text in cases where it's wider than the
> frame?

Worth considering. Note that fontsize may be dynamic e.g. `50+t*20` 
which can lead to jerky output if the text is resized mid-animation. I 
realize this should be the concern of the user, but so should fix_bounds 
in the first place :)


Regards,
Gyan
Gyan Jan. 19, 2018, 4:48 a.m. UTC | #3
On 1/11/2018 7:12 PM, Gyan Doshi wrote:
> The no-op commit e496c45 from Libav introduced an option which allowed 
> the user to relocate text to fit within the frame if it was going out of 
> bounds.
> 
> For some reason, when the merge commit b479e01 was applied, the option 
> was added but the code fragment which implemented it, was not. So the 
> option was dead on arrival, and has remained impotent (since Feb 2012).
> 
> Attached patch implements option. Text styling elements like shadow or 
> box are also kept within bounds. Default value changed to false so that 
> filter outcome doesn't change in existing scripts.
> 
> Regards,
> Gyan

Ping. Unless text needs to be rescaled as well, this patch is ready for 
review.

Regards,
Gyan
Kyle Swanson Jan. 19, 2018, 11:46 p.m. UTC | #4
Hi,

>
> Ping. Unless text needs to be rescaled as well, this patch is ready for review.
>
>
> Regards,
> Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


LGTM as-is. I'll push this sometime in the next 24 hours unless
someone else has anything to add.

Thanks,
Kyle
Michael Niedermayer Jan. 20, 2018, 1:03 a.m. UTC | #5
On Fri, Jan 19, 2018 at 03:46:57PM -0800, Kyle Swanson wrote:
> Hi,
> 
> >
> > Ping. Unless text needs to be rescaled as well, this patch is ready for review.
> >
> >
> > Regards,
> > Gyan
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> LGTM as-is. I'll push this sometime in the next 24 hours unless
> someone else has anything to add.

nothing to add, just wanted to test & push myself but as i saw your mail
and my todo is long i gladly leave that to you :)


[...]
Kyle Swanson Jan. 21, 2018, 12:37 a.m. UTC | #6
Hi,

On Fri, Jan 19, 2018 at 5:03 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Fri, Jan 19, 2018 at 03:46:57PM -0800, Kyle Swanson wrote:
>> Hi,
>>
>> >
>> > Ping. Unless text needs to be rescaled as well, this patch is ready for review.
>> >
>> >
>> > Regards,
>> > Gyan
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>> LGTM as-is. I'll push this sometime in the next 24 hours unless
>> someone else has anything to add.
>
> nothing to add, just wanted to test & push myself but as i saw your mail
> and my todo is long i gladly leave that to you :)
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Pushed. Thanks.

Kyle
diff mbox

Patch

diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index f6151443bb..f97a741b50 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -238,7 +238,7 @@  static const AVOption drawtext_options[]= {
     {"rate",            "set rate (timecode only)",         OFFSET(tc_rate),       AV_OPT_TYPE_RATIONAL, {.dbl=0},           0,  INT_MAX, FLAGS},
     {"reload",     "reload text file for each frame",                       OFFSET(reload),     AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
     { "alpha",       "apply alpha while rendering", OFFSET(a_expr),      AV_OPT_TYPE_STRING, { .str = "1"     },          .flags = FLAGS },
-    {"fix_bounds", "check and fix text coords to avoid clipping", OFFSET(fix_bounds), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS},
+    {"fix_bounds", "check and fix text coords to avoid clipping", OFFSET(fix_bounds), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
     {"start_number", "start frame number for n/frame_num variable", OFFSET(start_number), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX, FLAGS},
 
 #if CONFIG_LIBFRIBIDI
@@ -1401,6 +1401,32 @@  static int draw_text(AVFilterContext *ctx, AVFrame *frame,
     box_w = FFMIN(width - 1 , max_text_line_w);
     box_h = FFMIN(height - 1, y + s->max_glyph_h);
 
+    if (s->fix_bounds) {
+
+        /* calculate footprint of text effects */
+        int boxoffset     = s->draw_box ? FFMAX(s->boxborderw, 0) : 0;
+        int borderoffset  = s->borderw  ? FFMAX(s->borderw, 0) : 0;
+
+        int offsetleft = FFMAX3(boxoffset, borderoffset,
+                                (s->shadowx < 0 ? FFABS(s->shadowx) : 0));
+        int offsettop = FFMAX3(boxoffset, borderoffset,
+                                (s->shadowy < 0 ? FFABS(s->shadowy) : 0));
+
+        int offsetright = FFMAX3(boxoffset, borderoffset,
+                                 (s->shadowx > 0 ? s->shadowx : 0));
+        int offsetbottom = FFMAX3(boxoffset, borderoffset,
+                                  (s->shadowy > 0 ? s->shadowy : 0));
+
+
+        if (s->x - offsetleft < 0) s->x = offsetleft;
+        if (s->y - offsettop < 0)  s->y = offsettop;
+
+        if (s->x + box_w + offsetright > width)
+            s->x = FFMAX(width - box_w - offsetright, 0);
+        if (s->y + box_h + offsetbottom > height)
+            s->y = FFMAX(height - box_h - offsetbottom, 0);
+    }
+
     /* draw box */
     if (s->draw_box)
         ff_blend_rectangle(&s->dc, &boxcolor,