[MPlayer-dev-eng] [PATCH] SSA/ASS subtitles support

[Evgeniy Stepanov]

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"
> functions.
> 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.

Fixed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-ass.patch.bz2
Type: application/x-bzip2
Size: 23553 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060421/d04b18c8/attachment.bin>