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

[Reimar Döffinger]

Hello,
> Is the patch acceptable, and if not, what needs to be done ?

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.

The utf8 conversion functions (ass_utils.c) are, to be honest,
butt-ugly. But that is simple to fix later on. Though one comment
on it, why do you pass a pointer to utf8_charlen instead of just the char?

Two things I was wondering about but couldn't find out myself:
1) do the changes to demux_mkv.c cause any problems for the current
subtitle code? I.e. if this patch is applied, will there be any
differences between what the old and the new subtitle code support
except the extended things like color, positioning etc.?
2) how difficult would it be to make a vo support this? I think for this
it would be preferable if only the ass_draw_bitmap was in the filter
and the rendering would be done some place outside, with the
ass_draw_bitmap function being called via a VF_ message. Such a drawing
function would be easy to implement in the vos as well, and vf_vo could
pass such a message on to the vo if none of the filters accepted it.
Thus there shouldn't be any need for special cases.
Or is there too much data that the other functions currently in vf_ass
need and is only available there?

Greetings,
Reimar Döffinger