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

Gustavo Sverzut Barbieri barbieri
Sat Sep 23 18:26:29 CEST 2006


On 9/12/06, Gustavo Sverzut Barbieri <barbieri at gmail.com> wrote:
> 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.

any other objection?

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




More information about the ffmpeg-devel mailing list