[MPlayer-dev-eng] [PATCH] Tags support for SubRip and MicroDVD subtitles

[ubitux]

On Mon, May 17, 2010 at 10:34:50AM -0300, Kazuo Teramoto wrote:
> 2010/5/16 ubitux <ubitux at gmail.com>:
> > Hi,
> >
> > Well, I've spend my week end on supporting tags for SubRip and MicroDVD
> > subtitles, and it seems to work fine.
> >
> 
> Good work ubitux!
> 
> I'm testing the SubRip part of the code.
> 
> I think we need the support of named colors (from html 4.01 std). And
> a wrong attribute that is really common (from my subtitles sources at
> least), a color or size attribute withouts " (like color=#FFFFFF or
> color=white).

Really? They are skipping the quotes? :s

Well, if other players support it… 

> 
> And a behavior aspect, when its find an invalid attribute in font tag
> the parser stops, but I think that is best to continue parsing,
> because when can find a valid attr after that. For example
> 
> <font color="#00ffccerror" size="30">
> 
> Dont get a color, but do get a size.
> 

Well, I agree with supporting exotics syntaxes but the debate about how to
handle errors is simple in my mind: if it is unable in a way to parse the
font attributes, just display the full <font ...> as text so the user will
immediately see the error. Isn't it actually my code behaviour?

> I made a patch (to by applied after you patch) to solve that matters, in resume:
> - Named color (some code from Johatan patch)

Oh yeah I forgot it. Good one.

> - Attributes w/o " (size and color only)

I'm not sure this is a good idea. If we're starting to parse this we will
have to support single quotes, spaces after and before the '=', …

> - Continue parsing after a wrong attribute
> 

See upper.

> I used goto... sorry =[, I know goto is bad. But I tried to minimize
> the modification of your patch and mplayer.c use goto too (not really
> a excuse...). The patch is versus the git repo.
> 

I'm not against goto when they are well used, but I'm not the one who will
decide if the patch has to be accepted or not.

> Regards,
> Kazuo
> 

Well, thanks for your feedback. Also, I don't remember well but I think I
actually don't support multi lines tags in SubRip… So maybe I'll do some
work on it, but later.

There is also the subtitle position… but it needs other big changes.

-- 
ubitux