diff mbox series

[FFmpeg-devel] drawtext: Allow textfile path to be expanded per frame

Message ID 20200511143159.19390-1-david@andreoletti.net
State New
Delegated to: Michael Niedermayer
Headers show
Series [FFmpeg-devel] drawtext: Allow textfile path to be expanded per frame
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

David Andreoletti May 11, 2020, 2:31 p.m. UTC
drawtext allows a file to be reloaded per frame. However,
the file to be reloaded is constant across frame. With
textfile now supporting text expansion, a different file can
be reloaded per frame. Eg: textfile=/path/fo/file{frame_num}.txt

Signed-off-by: David Andreoletti <david@andreoletti.net>
---
 doc/filters.texi          | 16 +++++++++++++---
 libavfilter/vf_drawtext.c | 19 ++++++++++++++++---
 2 files changed, 29 insertions(+), 6 deletions(-)

Comments

David Andreoletti May 19, 2020, 3:43 a.m. UTC | #1
Hi ffmpeg maintainers / community,
New contributor here. The patch [0] for the drawtext filter was submitted some
time ago now but has been reviewed yet. It seems there is no official maintainer
for this filter (as per the MAINTAINERS file)
What should be done to have it reviewed ?

[0]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/

Regards,
David Andreoletti  





On Mon, May 11, 2020 2:31 PM, David Andreoletti david@andreoletti.net  wrote:
drawtext allows a file to be reloaded per frame. However,

the file to be reloaded is constant across frame. With

textfile now supporting text expansion, a different file can

be reloaded per frame. Eg: textfile=/path/fo/file{frame_num}.txt




Signed-off-by: David Andreoletti <david@andreoletti.net>

---

  doc/filters.texi | 16 +++++++++++++---

  libavfilter/vf_drawtext.c | 19 ++++++++++++++++---

  2 files changed, 29 insertions(+), 6 deletions(-)




diff --git a/doc/filters.texi b/doc/filters.texi

index d19fd346ae..460d65dd88 100644

--- a/doc/filters.texi

+++ b/doc/filters.texi

@@ -9659,13 +9659,13 @@ The default value of @var{borderw} is 0.

  Set the color to be used for drawing border around text. For the syntax of
this

  option, check the @ref{color syntax,,"Color" section in the ffmpeg-utils
manual,ffmpeg-utils}.

  

  The default value of @var{bordercolor} is "black".

  

  @item expansion

-Select how the @var{text} is expanded. Can be either @code{none},

+Select how the @var{text} and @var{textfile} are expanded. Can be either
@code{none},

  @code{strftime} (deprecated) or

  @code{normal} (default). See the @ref{drawtext_expansion, Text expansion}
section

  below for details.

  

  @item basetime

  Set a start time for the count. Value is in microseconds. Only applied

@@ -9786,15 +9786,19 @@ of UTF-8 encoded characters.

  

  This parameter is mandatory if no text string is specified with the

  parameter @var{text}.

  

  If both @var{text} and @var{textfile} are specified, an error is thrown.

  

+This parameter supports (per frame) variable expansion. Per frame variable 

+expansion requires @code{reload=1}. See @var{expansion} for details.

+

+

  @item reload

-If set to 1, the @var{textfile} will be reloaded before each frame.

-Be sure to update it atomically, or it may be read partially, or even fail.

+If set to 1, then before each frame, @var{textfile} file path will be expanded
and then the file at said path will be reloaded.

+Be sure to update the file atomically, or it may be read partially, or even
fail.

  

  @item x

  @item y

  The expressions which specify the offsets where text will be drawn

  within the video frame. They are relative to the top/left border of the

  output image.

@@ -10060,12 +10064,18 @@
drawtext="fontsize=15:fontfile=FreeSerif.ttf:text=LONG_LINE:y=h-line_h:x=-50*t"

  @item

  Show the content of file @file{CREDITS} off the bottom of the frame and scroll
up.

  @example

  drawtext="fontsize=20:fontfile=FreeSerif.ttf:textfile=CREDITS:y=h-20*t"

  @end example

  

+@item

+Each frame, reload a different text file at /path/to/frameX.txt, where X is
replaced with the frame number being processed by the filter

+@example

+drawtext="expansion:normal:textfile=/path/to/frame%@{frame_num@}.txt:reload=1"

+@end example

+

  @item

  Draw a single green letter "g", at the center of the input video.

  The glyph baseline is placed at half screen height.

  @example

 
drawtext="fontsize=60:fontfile=FreeSerif.ttf:fontcolor=green:text=g:x=(w-max_glyph_w)/2:y=h/2-ascent"

  @end example

diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c

index abe1ca6c35..ffb1ff2330 100644

--- a/libavfilter/vf_drawtext.c

+++ b/libavfilter/vf_drawtext.c

@@ -152,13 +152,14 @@ typedef struct DrawTextContext {

  AVBPrint expanded_text; ///< used to contain the expanded text

  uint8_t *fontcolor_expr; ///< fontcolor expression to evaluate

  AVBPrint expanded_fontcolor; ///< used to contain the expanded fontcolor spec

  int ft_load_flags; ///< flags used for loading fonts, see FT_LOAD_*

  FT_Vector *positions; ///< positions for each element in the text

  size_t nb_positions; ///< number of elements of positions array

- char *textfile; ///< file with text to be drawn

+ char *textfile; ///< filename with text to be drawn

+ AVBPrint expanded_textfile; ///< Same as textfile, except the filename can be
expanded

  int x; ///< x position to start drawing text

  int y; ///< y position to start drawing text

  int max_glyph_w; ///< max glyph width

  int max_glyph_h; ///< max glyph height

  int shadowx, shadowy;

  int borderw; ///< border width

@@ -565,24 +566,33 @@ static int load_font(AVFilterContext *ctx)

  if (!err)

  return 0;

  #endif

  return err;

  }

  

+static int expand_text(AVFilterContext *ctx, char *text, AVBPrint *bp);

  static int load_textfile(AVFilterContext *ctx)

  {

  DrawTextContext *s = ctx->priv;

  int err;

  uint8_t *textbuf;

  uint8_t *tmp;

  size_t textbuf_size;

  

- if ((err = av_file_map(s->textfile, &textbuf, &textbuf_size, 0, ctx)) < 0) {

+ if ((err = expand_text(ctx, s->textfile, &s->expanded_textfile)) < 0) {

+ av_log(ctx, AV_LOG_ERROR, "The text file path '%s' is not expandable\n",

+ s->textfile);

+ return err;

+ }

+

+ av_log(ctx, AV_LOG_DEBUG, "expanded_textfile:%s\n", s->expanded_textfile.str);

+

+ if ((err = av_file_map(s->expanded_textfile.str, &textbuf, &textbuf_size, 0,
ctx)) < 0) {

  av_log(ctx, AV_LOG_ERROR,

  "The text file '%s' could not be read or is empty\n",

- s->textfile);

+ s->expanded_textfile.str);

  return err;

  }

  

  if (textbuf_size > SIZE_MAX - 1 || !(tmp = av_realloc(s->text, textbuf_size +
1))) {

  av_file_unmap(textbuf, textbuf_size);

  return AVERROR(ENOMEM);

@@ -702,12 +712,14 @@ static av_cold int init(AVFilterContext *ctx)

  

  if (!s->fontfile && !CONFIG_LIBFONTCONFIG) {

  av_log(ctx, AV_LOG_ERROR, "No font filename provided\n");

  return AVERROR(EINVAL);

  }

  

+ av_bprint_init(&s->expanded_textfile, 0, AV_BPRINT_SIZE_UNLIMITED);

+

  if (s->textfile) {

  if (s->text) {

  av_log(ctx, AV_LOG_ERROR,

  "Both text and text file provided. Please provide only one\n");

  return AVERROR(EINVAL);

  }

@@ -820,12 +832,13 @@ static av_cold void uninit(AVFilterContext *ctx)

  FT_Done_Face(s->face);

  FT_Stroker_Done(s->stroker);

  FT_Done_FreeType(s->library);

  

  av_bprint_finalize(&s->expanded_text, NULL);

  av_bprint_finalize(&s->expanded_fontcolor, NULL);

+ av_bprint_finalize(&s->expanded_textfile, NULL);

  }

  

  static int config_input(AVFilterLink *inlink)

  {

  AVFilterContext *ctx = inlink->dst;

  DrawTextContext *s = ctx->priv;
Manolis Stamatogiannakis May 19, 2020, 9:34 a.m. UTC | #2
Hi David,

Not a full review, but a minor point that popped in mind.

It is common to pad serially numbered files with 0. E.g. if you have <1000
files, the 9th file will be named file009.txt.
Is this case handled by the expansion? Or is it assumed that the text file
numbers are not zero-padded?

If it the latter is the case, it would be good to make that explicit in the
documentation.
Of course, accounting for padding would be better, but this may not be
straightforward without adding more code.

Regards,
Manolis

On Tue, 19 May 2020 at 06:10, David Andreoletti <david@andreoletti.net>
wrote:

> Hi ffmpeg maintainers / community,
> New contributor here. The patch [0] for the drawtext filter was submitted
> some
> time ago now but has been reviewed yet. It seems there is no official
> maintainer
> for this filter (as per the MAINTAINERS file)
> What should be done to have it reviewed ?
>
> [0]
>
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/
>
> Regards,
> David Andreoletti
>
>
>
David Andreoletti May 19, 2020, 12:36 p.m. UTC | #3
Manolis: drawtext's text expansion section [0] does not mention the special
variable %{frame_num} has 0 paddable. When I tested locally (on master), it is
not 0 padded.

Running through different scenario, I recommend the expanded file path to not
contain 0 padded frame number:- if automatic padding is specified, then video
with no total frame known (live stream) would be problematic- if manually
padding is specified, it would complicate drawtext's textfile options parsing:
support for 0 padded %{frame_num} and possibly other custom formatting when
other supported %{....} variables are expanded.
For a first improvement to an existing functionality, I would prefer to keep it
simple and focused :-)

[0]https://ffmpeg.org/ffmpeg-filters.html#Text-expansion






On Tue, May 19, 2020 9:34 AM, Manolis Stamatogiannakis mstamat@gmail.com  wrote:
Hi David,




Not a full review, but a minor point that popped in mind.




It is common to pad serially numbered files with 0. E.g. if you have <1000

files, the 9th file will be named file009.txt.

Is this case handled by the expansion? Or is it assumed that the text file

numbers are not zero-padded?




If it the latter is the case, it would be good to make that explicit in the

documentation.

Of course, accounting for padding would be better, but this may not be

straightforward without adding more code.




Regards,

Manolis




On Tue, 19 May 2020 at 06:10, David Andreoletti <david@andreoletti.net>

wrote:




> Hi ffmpeg maintainers / community,

> New contributor here. The patch [0] for the drawtext filter was submitted

> some

> time ago now but has been reviewed yet. It seems there is no official

> maintainer

> for this filter (as per the MAINTAINERS file)

> What should be done to have it reviewed ?

>

> [0]

>

>
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/

>

> Regards,

> David Andreoletti

>

>

>
David Andreoletti May 25, 2020, 4:27 a.m. UTC | #4
FFmpeg team,
Is there any other discussion/changes needed to get this contribution merged in
?
Regards,  





On Tue, May 19, 2020 12:36 PM, David Andreoletti david@andreoletti.net  wrote:
Manolis: drawtext's text expansion section [0] does not mention the special
variable %{frame_num} has 0 paddable. When I tested locally (on master), it is
not 0 padded.

Running through different scenario, I recommend the expanded file path to not
contain 0 padded frame number:- if automatic padding is specified, then video
with no total frame known (live stream) would be problematic- if manually
padding is specified, it would complicate drawtext's textfile options parsing:
support for 0 padded %{frame_num} and possibly other custom formatting when
other supported %{....} variables are expanded.
For a first improvement to an existing functionality, I would prefer to keep it
simple and focused :-)

[0]https://ffmpeg.org/ffmpeg-filters.html#Text-expansion






On Tue, May 19, 2020 9:34 AM, Manolis Stamatogiannakis mstamat@gmail.com  wrote:
Hi David,




Not a full review, but a minor point that popped in mind.




It is common to pad serially numbered files with 0. E.g. if you have <1000

files, the 9th file will be named file009.txt.

Is this case handled by the expansion? Or is it assumed that the text file

numbers are not zero-padded?




If it the latter is the case, it would be good to make that explicit in the

documentation.

Of course, accounting for padding would be better, but this may not be

straightforward without adding more code.




Regards,

Manolis




On Tue, 19 May 2020 at 06:10, David Andreoletti <david@andreoletti.net>

wrote:




> Hi ffmpeg maintainers / community,

> New contributor here. The patch [0] for the drawtext filter was submitted

> some

> time ago now but has been reviewed yet. It seems there is no official

> maintainer

> for this filter (as per the MAINTAINERS file)

> What should be done to have it reviewed ?

>

> [0]

>

>
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/

>

> Regards,

> David Andreoletti

>

>

>
David Andreoletti July 1, 2020, 11:08 p.m. UTC | #5
**Help needed**: I am still hopeful the patch will get some attention but hope
is fading now (due to summer) :-). It's an easy patch and probably takes
10-20min to review.
Quick patch's status:- it compiles on the latest ffmpeg 4.3,- it passes the
tests,- the associated documentation has been updated with examples (and
documentation builds successfully),- Manolis Stamatogiannakis gave feedback, to
which I replied. No more action on the feedback needed as far as I can tell.-
The patch's age is ~ 2 months and awaiting someone to make it progress (feedback
or merge).
On pathwork [0], a few weeks ago, I assigned it to Michael Niedermayer as a last
resort but I was told on IRC he is super busy.[0]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/
Any volunteer :-) ?






On Mon, May 25, 2020 4:27 AM, David Andreoletti david@andreoletti.net  wrote:
FFmpeg team,
Is there any other discussion/changes needed to get this contribution merged in
?
Regards,  





On Tue, May 19, 2020 12:36 PM, David Andreoletti david@andreoletti.net  wrote:
Manolis: drawtext's text expansion section [0] does not mention the special
variable %{frame_num} has 0 paddable. When I tested locally (on master), it is
not 0 padded.

Running through different scenario, I recommend the expanded file path to not
contain 0 padded frame number:- if automatic padding is specified, then video
with no total frame known (live stream) would be problematic- if manually
padding is specified, it would complicate drawtext's textfile options parsing:
support for 0 padded %{frame_num} and possibly other custom formatting when
other supported %{....} variables are expanded.
For a first improvement to an existing functionality, I would prefer to keep it
simple and focused :-)

[0]https://ffmpeg.org/ffmpeg-filters.html#Text-expansion






On Tue, May 19, 2020 9:34 AM, Manolis Stamatogiannakis mstamat@gmail.com  wrote:
Hi David,




Not a full review, but a minor point that popped in mind.




It is common to pad serially numbered files with 0. E.g. if you have <1000

files, the 9th file will be named file009.txt.

Is this case handled by the expansion? Or is it assumed that the text file

numbers are not zero-padded?




If it the latter is the case, it would be good to make that explicit in the

documentation.

Of course, accounting for padding would be better, but this may not be

straightforward without adding more code.




Regards,

Manolis




On Tue, 19 May 2020 at 06:10, David Andreoletti <david@andreoletti.net>

wrote:




> Hi ffmpeg maintainers / community,

> New contributor here. The patch [0] for the drawtext filter was submitted

> some

> time ago now but has been reviewed yet. It seems there is no official

> maintainer

> for this filter (as per the MAINTAINERS file)

> What should be done to have it reviewed ?

>

> [0]

>

>
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200511143159.19390-1-david@andreoletti.net/

>

> Regards,

> David Andreoletti

>

>

>
diff mbox series

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index d19fd346ae..460d65dd88 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -9659,13 +9659,13 @@  The default value of @var{borderw} is 0.
 Set the color to be used for drawing border around text. For the syntax of this
 option, check the @ref{color syntax,,"Color" section in the ffmpeg-utils manual,ffmpeg-utils}.
 
 The default value of @var{bordercolor} is "black".
 
 @item expansion
-Select how the @var{text} is expanded. Can be either @code{none},
+Select how the @var{text} and @var{textfile} are expanded. Can be either @code{none},
 @code{strftime} (deprecated) or
 @code{normal} (default). See the @ref{drawtext_expansion, Text expansion} section
 below for details.
 
 @item basetime
 Set a start time for the count. Value is in microseconds. Only applied
@@ -9786,15 +9786,19 @@  of UTF-8 encoded characters.
 
 This parameter is mandatory if no text string is specified with the
 parameter @var{text}.
 
 If both @var{text} and @var{textfile} are specified, an error is thrown.
 
+This parameter supports (per frame) variable expansion. Per frame variable 
+expansion requires @code{reload=1}. See @var{expansion} for details.
+
+
 @item reload
-If set to 1, the @var{textfile} will be reloaded before each frame.
-Be sure to update it atomically, or it may be read partially, or even fail.
+If set to 1, then before each frame, @var{textfile} file path will be expanded and then the file at said path will be reloaded.
+Be sure to update the file atomically, or it may be read partially, or even fail.
 
 @item x
 @item y
 The expressions which specify the offsets where text will be drawn
 within the video frame. They are relative to the top/left border of the
 output image.
@@ -10060,12 +10064,18 @@  drawtext="fontsize=15:fontfile=FreeSerif.ttf:text=LONG_LINE:y=h-line_h:x=-50*t"
 @item
 Show the content of file @file{CREDITS} off the bottom of the frame and scroll up.
 @example
 drawtext="fontsize=20:fontfile=FreeSerif.ttf:textfile=CREDITS:y=h-20*t"
 @end example
 
+@item
+Each frame, reload a different text file at /path/to/frameX.txt, where X is replaced with the frame number being processed by the filter
+@example
+drawtext="expansion:normal:textfile=/path/to/frame%@{frame_num@}.txt:reload=1"
+@end example
+
 @item
 Draw a single green letter "g", at the center of the input video.
 The glyph baseline is placed at half screen height.
 @example
 drawtext="fontsize=60:fontfile=FreeSerif.ttf:fontcolor=green:text=g:x=(w-max_glyph_w)/2:y=h/2-ascent"
 @end example
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index abe1ca6c35..ffb1ff2330 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -152,13 +152,14 @@  typedef struct DrawTextContext {
     AVBPrint expanded_text;         ///< used to contain the expanded text
     uint8_t *fontcolor_expr;        ///< fontcolor expression to evaluate
     AVBPrint expanded_fontcolor;    ///< used to contain the expanded fontcolor spec
     int ft_load_flags;              ///< flags used for loading fonts, see FT_LOAD_*
     FT_Vector *positions;           ///< positions for each element in the text
     size_t nb_positions;            ///< number of elements of positions array
-    char *textfile;                 ///< file with text to be drawn
+    char *textfile;                 ///< filename with text to be drawn
+    AVBPrint expanded_textfile;     ///< Same as textfile, except the filename can be expanded
     int x;                          ///< x position to start drawing text
     int y;                          ///< y position to start drawing text
     int max_glyph_w;                ///< max glyph width
     int max_glyph_h;                ///< max glyph height
     int shadowx, shadowy;
     int borderw;                    ///< border width
@@ -565,24 +566,33 @@  static int load_font(AVFilterContext *ctx)
     if (!err)
         return 0;
 #endif
     return err;
 }
 
+static int expand_text(AVFilterContext *ctx, char *text, AVBPrint *bp);
 static int load_textfile(AVFilterContext *ctx)
 {
     DrawTextContext *s = ctx->priv;
     int err;
     uint8_t *textbuf;
     uint8_t *tmp;
     size_t textbuf_size;
 
-    if ((err = av_file_map(s->textfile, &textbuf, &textbuf_size, 0, ctx)) < 0) {
+    if ((err = expand_text(ctx, s->textfile, &s->expanded_textfile)) < 0) {
+        av_log(ctx, AV_LOG_ERROR, "The text file path '%s' is not expandable\n",
+            s->textfile);
+        return err;
+    }
+
+    av_log(ctx, AV_LOG_DEBUG, "expanded_textfile:%s\n", s->expanded_textfile.str);
+
+    if ((err = av_file_map(s->expanded_textfile.str, &textbuf, &textbuf_size, 0, ctx)) < 0) {
         av_log(ctx, AV_LOG_ERROR,
                "The text file '%s' could not be read or is empty\n",
-               s->textfile);
+               s->expanded_textfile.str);
         return err;
     }
 
     if (textbuf_size > SIZE_MAX - 1 || !(tmp = av_realloc(s->text, textbuf_size + 1))) {
         av_file_unmap(textbuf, textbuf_size);
         return AVERROR(ENOMEM);
@@ -702,12 +712,14 @@  static av_cold int init(AVFilterContext *ctx)
 
     if (!s->fontfile && !CONFIG_LIBFONTCONFIG) {
         av_log(ctx, AV_LOG_ERROR, "No font filename provided\n");
         return AVERROR(EINVAL);
     }
 
+    av_bprint_init(&s->expanded_textfile, 0, AV_BPRINT_SIZE_UNLIMITED);
+
     if (s->textfile) {
         if (s->text) {
             av_log(ctx, AV_LOG_ERROR,
                    "Both text and text file provided. Please provide only one\n");
             return AVERROR(EINVAL);
         }
@@ -820,12 +832,13 @@  static av_cold void uninit(AVFilterContext *ctx)
     FT_Done_Face(s->face);
     FT_Stroker_Done(s->stroker);
     FT_Done_FreeType(s->library);
 
     av_bprint_finalize(&s->expanded_text, NULL);
     av_bprint_finalize(&s->expanded_fontcolor, NULL);
+    av_bprint_finalize(&s->expanded_textfile, NULL);
 }
 
 static int config_input(AVFilterLink *inlink)
 {
     AVFilterContext *ctx = inlink->dst;
     DrawTextContext *s = ctx->priv;