[MPlayer-dev-eng] [PATCH] SSA/ASS subtitles support
eugeni.stepanov at gmail.com
Fri Apr 21 02:12:20 CEST 2006
On Wednesday 19 April 2006 17:59, Reimar Döffinger wrote:
> Very quick review:
> > -D_GNU_SOURCE \
> > + $(FREETYPE_INC)
> There should be a \ at the end of the line as well (so that future
> patches do not need to change this line).
> Code should be documentated as described in
> DOCS/tech/code-documentation.txt, i.e. doxygen comments at least for
> every global function/variable, and IMHO also for local (i.e. "static"
> Well, it probably doesn't make sense to comment the default
> filter-functions, but I guess you get the idea ;-)
> You should not mix tabs and space for indentation (ass_style_t in
> libsub/ass_types.h looks particularly bad) - I personally prefer spaces,
> they are IMO less likely to break, but that's you decision.
> Variable declarations must be at the beginning of a block (see e.g.
> ass_draw_bitmap, will not compile with gcc-2.95 otherwise).
> The stub functions in ass.c might be cleaner to do via defines in
> ass.h (wouldn't cause useless function calls etc.), not sure about
> it though.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 23553 bytes
Desc: not available
More information about the MPlayer-dev-eng