[MPlayer-dev-eng] [PATCH] improvement of sami subtitles feature

[Clément Bœsch]

On Fri, Feb 15, 2013 at 12:57:06PM +0900, Jihyun Cho wrote:
> 2013/2/15 Clément Bœsch <ubitux at gmail.com>
> 
> > On Thu, Feb 14, 2013 at 04:18:16PM +0900, Jihyun Cho wrote:
> > [...]
> > > I separated and trimmed the patch.
> > > 'mplayer_sami_ass_converter.patch' depends on
> > > 'mplayer_sami_html_character_entities.patch', because it uses
> > sami_entities
> > > table.
> >
> > AFAICT you are duplicating the SubRip-to-ASS code for the insane idea of
> > supporting the SubRip markup into SAMI. Maybe because some weird converter
> > copy/pasted the dialogues from a .srt to a .sami without taking into
> > account the markup at all and so you got a broken file? Assuming this, we
> > should support the SubRip markup in all the subtitles formats, which is
> > wrong.
> >
> > I'm not really implicated in MPlayer development anymore so I won't object
> > into its inclusion, but it is IMO not correct. Also, the patch is
> > duplicating a lot of code which is a maintenance nightmare.
> >
> > On the other hand, the second patch (HTML codes mapping) is more
> > interesting, and it might make sense to propose something similar into the
> > SAMI decoder of FFmpeg.
> >
> > Now in an ideal world, MPlayer should replace the subreader.c with the
> > subtitles support in FFmpeg (now that it supports every subtitles formats
> > MPlayer has) and as a consequence remove the sub-ass-converting code I
> > added a while ago as a "good enough temporary solution" (aka "clean hack")
> > which won't be necessary anymore (the decoding process of FFmpeg subtitles
> > is outputting ASS for every formats).
> >
> > Though, I see a few blocking things for this to happen in MPlayer:
> >  - FFmpeg doesn't support character encoding convert yet, but I'm working
> >    on it, and I expect this to be solved soon
> >  - FFmpeg doesn't support UTF-16, this is a problem which could be somehow
> >    work arounded in MPlayer given a few efforts, or better, fixed in
> >    FFmpeg.
> >  - The subtitles "dumping" code can't be replaced with FFmpeg API because
> >    there is not enough subtitles encoders, so it will mean dropping
> >    features
> >  - It will require dropping a lot of code in MPlayer, and the development
> >    of MPlayer being somehow slow lately, it might require some motivation
> >    for the developer trying to that work.
> >
> > Anyway, the idea is that adding such code in MPlayer might be a pointless
> > effort because one of the purpose of FFmpeg is actually to lighten up the
> > burden of dealing with formats & codecs in MPlayer, including subtitles.
> > This means whatever code you add in MPlayer for subtitles, it might
> > disappear in a relatively short amount of time because of FFmpeg. So if
> > you want your code to live on longer, I think it's wise to work on the
> > subtitles support in FFmpeg instead. You might also get more insightful
> > reviews about subtitles there since there is more activity.
> >
> > Now all I just said is relevant only if MPlayer moves to FFmpeg subtitles
> > decoding at some point. While I believe it will happen in one or two
> > MPlayer forks, I'm not so sure about the original project, so this is up
> > to you to decide.
> >
> > Regards,
> >
> 
> As you said, Sami-to-ASS converter is temporary solution.
> It has a lot of duplicated code with SubRip-to-ASS converter.
> I think Sami-to-ASS converter can be merged to SubRip-to-ASS.
> But Sami spec is not the same as SubRip spec. So I decided to separate two
> converters.

Yes. But the question was why are you trying to support SubRip markup into
SAMI?

> As a result, a lot of code is duplicated.
> I know it's not good solution. But it works.
> 

> On the other hand, I tried converting Sami to ASS with FFmpeg.
> But it is not enough to replace mplayer's subreader.c right now.

If not (good?) enough? Or you meant something else?

> In the long term, I will look into FFmpeg code.
> 
> Thanks for the comment,
> 
> Jihyun

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20130215/3fae4fac/attachment.asc>