[FFmpeg-devel] aalib video filter patch review request

[Stefano Sabatini]

On date Sunday 2014-06-29 19:13:39 +0400, iamtakingiteasy at eientei.org encoded:
> Thank you, Stefano Sabatini and Michael Niedermayer for your
> replies, i have considered your notes and re-written aalib filter
> according to them.
> 
> This results in three commits, which will follow this message, as
> stated in [1] keeping them separate simplifies reviewing process:
> 
> * [lavfi][aalib][PATCH] lavfi: Generalization of freetype/fontconfig
> font loading.
> * [lavfi][aalib][PATCH] lavfi: Proposed changes to vf_drawtext
> filter to use new fontconfig utils.
> * [lavfi][aalib][PATCH] lavfi: add aalib video filter.
> 
> The first patch is not altering anything functional, but allows to
> second and third patch to be applied.
> The main idea is to extract common parts (FreeType2 and Fontconfig)
> of font discovery and loading from vf_aa and vf_drawtext and place
> them into freetypeutils.h/c files.
> 
> The second patch is, as far i understand, should be maintained by
> some other person, but i wasn't able to find who is in charge for
> vf_drawtext files by looking at MAINTAINERS file, yet still it is
> only a suggestion how to change vf_drawtext.c in order to
> incorporate new freetype/fontconfig mechanism.
> 
> And finally the last patch is the rewritten vf_aa.c.
> Answering to question of Stefano Sabatini:
> 

> >>+static struct aa_driver vf_driver = {
> >>+    .shortname = "vf",
> >>+    .name = "video filter driver",
> >>+    .init = vf_driver_init,
> >>+    .uninit = vf_driver_uninit,
> >>+    .setattr = vf_driver_setattr,
> >>+    .getsize = vf_driver_getsize,
> >>+    .print = vf_driver_print,
> >>+    .gotoxy = vf_driver_gotoxy
> >>+};
> 
> >what's this good for?
> 
> This is a aa_driver custom driver descriptor, as aa_driver' methods
> are only means to actually receive any updates on aa-processed
> framebuffer.

The question is, it is actually used? Is it used for testing, or is
required by the other filter? Also "vf" is a too generic name.

> 
> --
> 

> Also, one open question is the libavfilter/version.h incrementing,
> not sure how all this three patches should operate on this matter.
> Currently they're all setting minor version to 10 (+1), but possibly
> application of each one should increment either it or micro version
> of lavfi, please clarify.

You can leave this to the committer, minor bump is required when we
add a new component to the filter, simple refactoring does not require
minor bumping (but we sometimes increment micro).
-- 
FFmpeg = Faithless and Fascinating Meaningful Philosofic Elegant Gem