[MPlayer-dev-eng] [RFC] subtitle cleanup

[Evgeniy Stepanov]

On Friday 22 December 2006 20:19, Reimar Döffinger wrote:
> > > > This looks wrong, now if libass rendering for non-SSA subtitles is
> > > > enabled they'd be rendered twice. Either the libass rendering case
> > >
> > > Actually I think the clear_subtitle from above should just be moved
> > > into the ass_enabled (even without this patch I think this would make
> > > more sense, at least if it works, I did not test)
> >
> > That wouldn't work. You're leaving both the libass and non-libass
> > rendering enabled in the patch; when libass rendering is wanted the
> > non-libass version must be disabled, and moving the line wouldn't do
> > that. I still think the best way to do that would be to move the "use
> > libass for non-SSA rendering" logic to the common subtitle handler.
>
> Huh? Moving the line means that when ass_enabled is set then at the
> start of the function vo_sub->lines will be 0 and afterwards it will be
> 0, too. So no subtitle displayed. Of course, setting vo_sub = NULL in
> addition would be possible, too.

No, afterwards it will contain the last subtitle and should be set to NULL.

> Or giving these two function in sub.c an additional subtitle * argument,
> doing that seemed a bit obfuscating to me at first though.

I like this. vo_sub_add_text() in your patch does 2 unrelated things: parsing 
the subtitle text and setting vo_sub. I'd prefer them separated. This will 
also make the code in demux_mkv clearer, I think.

And I agree with Uoti, it has nothing to do with vo, because it's more about 
parsing subtitles than displaying them.