[Ffmpeg-devel] [PATCH] rewrite vhook/drawtext.c

[Gustavo Sverzut Barbieri]

On 9/6/06, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
> On Tue, Sep 05, 2006 at 11:25:59PM -0300, Gustavo Sverzut Barbieri wrote:
> > Ok, based on comments, here is the second try, changes:
> >
> > - err() and warn() -> av_log()
>
> no its stil full of err, fprintf, fputc and so on

Ouch! Ouch! Ouch! My bad!

I did a mistake before making the new diff, here follows the correct version.


> > - makefile: use vhook specific LDFLAGS by appending library name with
> > $(LDFLAGS_$@) and defining LDFLAGS_drawtext.so. I'm not sure if this
> > works with every build system, please check.
>
> could you summarize the difference between the old and rewritten drawtext.c
>
> also if the files have nothing in common then replacing the old by the new
> will not be accepted, rather add the new as drawtext2.c and disable the
> old but not remove it, if then after a few month there are no complaints
> then the old could be removed

no, essentially they're the same thing. Here follows the set of changes:
 - move macros to inline: less error-prone. ie: the old macros did use
{} blocks, instead of do { } while (0);... which is buggy if used with
"if" clauses. This is for GET_PIXEL ->get_pixel, SET_PIXEL ->
set_pixel, RGB_TO_YUV -> rgb_to_yuv
 - replace time() usage, use gettimeofday()
 - use posix_fadvise() to hint about file usage.
 - use stdint types
 - isolate code in smaller functions, this helps understanding what
code is doing, even things like "value >> 6" are not trivial to
understand (why divide by 64?), but inside a function
"ft_pos_to_pixel()" it make clear it's because you want to convert
freetype position to pixel units.

Aside from this refactoring, I did fix some algorithm bugs with
draw_text(), the old way was complicated and did work for my common
case, but not for every case.... background drawing was specially
flawed, since the limits detection were wrong. Border drawing also
were overcomplicated, it's simpler now. Function measure_and_cache()
is what you want to read for position and size calculation (this is
used by draw_text and draw_box).

I did wrote the code and it was pending a rewrite since then. Now I
had some time and here it is.

I hope you understand,


-- 
Gustavo Sverzut Barbieri
--------------------------------------
Jabber: barbieri at gmail.com
   MSN: barbieri at gmail.com
  ICQ#: 17249123
 Skype: gsbarbieri
Mobile: +55 (81) 9927 0010
 Phone:  +1 (347) 624 6296; 08122692 at sip.stanaphone.com
   GPG: 0xB640E1A2 @ wwwkeys.pgp.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: drawtext-rewrite3.patch
Type: text/x-patch
Size: 27850 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20060906/1036b88a/attachment.bin>