[MPlayer-dev-eng] [PATCH] Review please: support sub_pos in libass

[Evgeniy Stepanov]

On Tuesday 23 December 2008 01:01:57 Uoti Urpala wrote:
> On Mon, 2008-12-22 at 21:48 +0100, Rafał Miłecki wrote:
> > Hope this time I'll provide some useful patch :)
> >
> > Currently it is almost not possible to control position of subtitles
> > when using libass. OK, libass respects position from .ass file but
>
> So this is primarily for rendering plain non-ass subtitle formats with
> libass?
>
> > This patch does:
> > 1) Change default Alignment to 14 (0x1110) which is /invalid valign/
> > and HALIGN_CENTER.

You'd better give it a name, like VALIGN_PERCENTPOS, and keep that "else" 
branch for traditional ASS subtitles (VALIGN_SUB). Less likely to break 
something this way.

> > 2) Respects sub_pos in case of invalid value of (Alignment & 0x1100)
>
> At least the position value should be set in the style struct rather
> than read from a global. Especially so because libass is used in other
> projects and should not use MPlayer-specific globals.

Yes.

> One possibility would be to add an alignment type that reads the exact
> position from another variable in the struct,

Probably percentage of image height, not the exact pixel position. Also 
(unrelated), it is not an exact position in the sense of \pos(): the subtitle, 
if needed, will be moved up or down to fit the screen instead of being clipped 
by the screen border.

> and then allow replacing
> the default style with another (would that be any harder than changing
> just the alignment?).

I think we should have a global set of styles for plaintext tracks.When subs 
are switched to another plaintext track, new track's styles are replaced with 
a copy of the global set. Updates from events go to both the global set and 
the current track's styles.

> The preferred interface could depend on how libass is used in other
> projects; I'm not too familiar with those.

I don't see a need for any new interface, except for the new variable in style 
struct and a new VALIGN_ constant. And maybe some flag telling that this a 
plaintext track, not a real ASS (to handle track switching). Most of the code 
will go to ass_mp.c, which is mplayer-specific.