[FFmpeg-devel] [PATCH] drawtext: fix hard dependency to lavc (timecode).

[Clément Bœsch]

On Wed, Dec 28, 2011 at 01:28:42PM +0100, Stefano Sabatini wrote:
> On date Wednesday 2011-12-28 10:25:02 +0100, Clément Bœsch encoded:
> > From: Clément Bœsch <clement.boesch at smartjog.com>
> > 
> > ---
> > As Stefano proposed, we might want to move the timecode stuff to lavu at some
> > point. But right now, since there are already some function renames waiting for
> > the next bump, I'd wait until then. Also, the API might not be mature enough to
> > be exported properly, thus we might reconsider when the timecode support is
> > more complete.
> > ---
> >  libavfilter/vf_drawtext.c |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> > index 344c8db..fc0fcc4 100644
> > --- a/libavfilter/vf_drawtext.c
> > +++ b/libavfilter/vf_drawtext.c
> > @@ -29,8 +29,14 @@
> >  #include <sys/time.h>
> >  #include <time.h>
> >  
> > +#include "config.h"
> > +
> > +#if CONFIG_AVCODEC
> > +/* timecode related includes */
> >  #include "libavcodec/timecode.h"
> >  #include "libavutil/avstring.h"
> > +#endif
> > +
> >  #include "libavutil/colorspace.h"
> >  #include "libavutil/file.h"
> >  #include "libavutil/eval.h"
> > @@ -156,8 +162,10 @@ typedef struct {
> >      AVExpr *d_pexpr;
> >      int draw;                       ///< set to zero to prevent drawing
> >      AVLFG  prng;                    ///< random
> > +#if CONFIG_AVCODEC
> >      struct ff_timecode tc;
> >      int frame_id;
> > +#endif
> >  } DrawTextContext;
> >  
> >  #define OFFSET(x) offsetof(DrawTextContext, x)
> > @@ -178,9 +186,11 @@ static const AVOption drawtext_options[]= {
> >  {"tabsize",  "set tab size",         OFFSET(tabsize),            AV_OPT_TYPE_INT,    {.dbl=4},     0,        INT_MAX  },
> >  {"basetime", "set base time",        OFFSET(basetime),           AV_OPT_TYPE_INT64,  {.dbl=AV_NOPTS_VALUE},     INT64_MIN,        INT64_MAX  },
> >  {"draw",     "if false do not draw", OFFSET(d_expr),             AV_OPT_TYPE_STRING, {.str="1"},   CHAR_MIN, CHAR_MAX },
> > +#if CONFIG_AVCODEC
> >  {"timecode", "set initial timecode", OFFSET(tc.str),             AV_OPT_TYPE_STRING, {.str=NULL},  CHAR_MIN, CHAR_MAX },
> >  {"r",        "set rate (timecode only)", OFFSET(tc.rate),        AV_OPT_TYPE_RATIONAL, {.dbl=0},          0,  INT_MAX },
> >  {"rate",     "set rate (timecode only)", OFFSET(tc.rate),        AV_OPT_TYPE_RATIONAL, {.dbl=0},          0,  INT_MAX },
> > +#endif
> >  
> >  /* FT_LOAD_* flags */
> >  {"ft_load_flags", "set font loading flags for libfreetype",   OFFSET(ft_load_flags),  AV_OPT_TYPE_FLAGS,  {.dbl=FT_LOAD_DEFAULT|FT_LOAD_RENDER}, 0, INT_MAX, 0, "ft_load_flags" },
> > @@ -340,16 +350,23 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> >          av_file_unmap(textbuf, textbuf_size);
> >      }
> >  
> > +#if CONFIG_AVCODEC
> >      if (dtext->tc.str) {
> >          if (avpriv_init_smpte_timecode(ctx, &dtext->tc) < 0)
> >              return AVERROR(EINVAL);
> >          if (!dtext->text)
> >              dtext->text = av_strdup("");
> >      }
> > +#endif
> >  
> >      if (!dtext->text) {
> > +#if CONFIG_AVCODEC
> >          av_log(ctx, AV_LOG_ERROR,
> >                 "Either text, a valid file or a timecode must be provided\n");
> > +#else
> > +        av_log(ctx, AV_LOG_ERROR,
> > +               "Either text or a valid file must be provided\n");
> > +#endif
> 
> nit: maybe factorize the first line
> 

I'd prefer to keep the parenthesis matching if you don't mind.

> >          return AVERROR(EINVAL);
> >      }
> >  
> > @@ -722,11 +739,13 @@ static int draw_text(AVFilterContext *ctx, AVFilterBufferRef *picref,
> >          buf_size *= 2;
> >      } while ((buf = av_realloc(buf, buf_size)));
> >  
> > +#if CONFIG_AVCODEC
> >      if (dtext->tc.str) {
> >          char tcbuf[sizeof("hh:mm:ss.ff")];
> >          avpriv_timecode_to_string(tcbuf, &dtext->tc, dtext->frame_id++);
> >          buf = av_asprintf("%s%s", dtext->text, tcbuf);
> >      }
> > +#endif
> >  
> >      if (!buf)
> >          return AVERROR(ENOMEM);
> 
> Looks good to me, but please mention it in the docs for
> completeness-sake, in the unlikely case an user won't compile
> libavfilter against libavcodec, something along the lines of:
> 
> | The @option{timecode}, @option{t}, and @option{rate} options are
> | only available if libavfilter was built with libavcodec enabled.
> 
> Alternatively you may consider a run-time error issued in case one of
> those options is called and CONFIG_AVCODEC was not enabled.
> 

Is it ok to include libavcodec stuff and just avoid calling functions when
it is disabled? (I "need" it for the ff_timecode struct to load the
timecode string option). If so I'll resend a patch based on Reimar's
proposition and yours.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111229/82254642/attachment.asc>