[FFmpeg-soc] libavfilter audio work - qualification task

Víctor Paesa victorpaesa at googlemail.com
Sat Apr 10 11:04:18 CEST 2010


Hi,

A few cosmetics/comments on your code:

> Attached is drawtext.diff which contains the libavfilter folder Makefile
> change, allfilters.c change and vf_drawtext.c

> Index: allfilters.c
> ===================================================================
> --- allfilters.c      (revision 5734)
> +++ allfilters.c      (working copy)
> @@ -54,6 +54,7 @@
>      REGISTER_FILTER (SPLIT,       split,       vf);
>      REGISTER_FILTER (TRANSPOSE,   transpose,   vf);
>      REGISTER_FILTER (VFLIP,       vflip,       vf);
> +    REGISTER_FILTER (DRAWTEXT,    drawtext,    vf);
>
>      REGISTER_FILTER (BUFFER,      buffer,      vsrc);
>      REGISTER_FILTER (MOVIE,       movie,       vsrc);
> Index: Makefile
> ===================================================================
> --- Makefile  (revision 5734)
> +++ Makefile  (working copy)
> @@ -35,6 +35,7 @@
>  OBJS-$(CONFIG_SPLIT_FILTER)                  += vf_split.o
>  OBJS-$(CONFIG_TRANSPOSE_FILTER)              += vf_transpose.o
>  OBJS-$(CONFIG_VFLIP_FILTER)                  += vf_vflip.o
> +OBJS-$(CONFIG_DRAWTEXT_FILTER)               += vf_drawtext.o
>
>  OBJS-$(CONFIG_BUFFER_FILTER)                 += vsrc_buffer.o
>  OBJS-$(CONFIG_MOVIE_FILTER)                  += vsrc_movie.o

Please keep filters in alphabetic order.

+ ******************************************************************************
+ * Original vhook author: Gustavo Sverzut Barbieri <gsbarbieri ....
+ * Libavfilter version  : S.N. Hemanth Meenakshisundaram <smeenaks ...

You may want to obfuscate the mail addresses to avoid feeding spammers.

+ * - Line Wrap (if the text doesn't fit, the next char go to the next line)

Docs use formal style: "does not"


+    /* load and cache glyphs */
+    yMax = -32000;
+    yMin =  32000;
+    for (c=0; c < 256; c++)
+    {
+        /* Load char */

If -T is not used, you need to cache only the different chars existing
in the -t argument (much less than 255 probably).

Also, I would say in docs if Unicode text is supported.
And I would add a FIXME comment to code if not.

Regards,
Víctor


More information about the FFmpeg-soc mailing list