[Ffmpeg-devel] [PATCH] drawtext.c: 07fix glyphs cache and ft_pos_to_pixels

[Gustavo Sverzut Barbieri]

On 9/12/06, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
> On Sun, Sep 10, 2006 at 03:55:12PM -0300, Gustavo Sverzut Barbieri wrote:
> > This fix glyphs cache when it cannot load character (fix possible
> > access to non-initialized memory) an declare ft_pos_to_pixels()
> > function.
> >
> > it's simple, but seems long since code was moved into another block
> > (and thus indentation level) due an "if" clause.
>
> to quote our policy:
> "NOTE: If you had to put if(){ .. } over a large (> 5 lines) chunk of code,
> then either do NOT change the indentation of the inner part within (don't
> move it to the right)! or do so in a separate commit"

I know what you mean, but given this patch size, are this really required?


> [...]
> > +/**
> > + * FreeType represent values in 64/th of pixels.
> > + */
> > +static inline int ft_pos_to_pixels(FT_Pos value)
> > +{
> > +  return value >> 6;
> > +}
> > +
>
> is this really needed? if so why dont you replace every a+b by add(a,b) ?
> anyway is the rounding correct at all?

I put this inside a function because I was lost when I read my own
code on why 64... this make it clear... and it have nothing to do with
add(a,b) since you have a constant... maybe add_b(a) would be more
likely to express what you mean. I would keep it.

as if it's ok or not, I just followed FreeType examples when writing this.


-- 
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