[vlc-devel] Crystal HD for 1080i MPEG2 and closed captions on low-end system

[Jean-Baptiste Kempf]

Hello,

On Fri, Jun 10, 2011 at 10:06:18AM -0400, Devin Heitmueller wrote :
> If you have specific concerns, I would be interested in hearing them.

The part hardcoding "DejaVu Sans Mono" was the one that worried me a
bit, especially since since there is already some ugliness on freetype.c
about this and that most distribution already change it there.

> There were definitely some cases where the code is ugly, largely due
> to limitations of the VLC framework and pre-existing code.

I am not blaming you, really not ;)

> > However, the freetype module has seen some major rework and I think
> > Laurent is changing quite a bit how the rendering is done, so I think
> > the blackbox commit will not be mergeable.
> 
> This doesn't surprise me.  I definitely would have preferred to avoid
> changing the Freetype module at all, but it was unfortunately
> necessary in order to do proper layout of the captions (as well as
> introducing the black background behind the text).

I understood this in your commits and your changes are minor there,
except one commit that should be unnecessary once Laurent will finish
his code.

> > Just two remarks, you are allowed to use C99 for in VLC, and you should
> > really check your indentation (4 spaces) for VLC code.
> 
> Yup, my focus was on delivering functional patches, and I hadn't yet
> taken a pass over the patches to address codingstyle issue.

This is fully understood, and we all know the timing necessity that
happen IRL. My comments were more for upstream inclusion, because it
would be stupid not to merge your work, IMVHO.

Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device