[MPlayer-cvslog] r32035 - in branches/1.0rc4: . libmpdemux/demux_lavf.c

[Reimar Döffinger]

On Wed, Sep 01, 2010 at 04:42:02PM +0200, Aurelien Jacobs wrote:
> On Wed, Sep 01, 2010 at 08:05:27AM +0200, Reimar Döffinger wrote:
> > Will you implement it?
> 
> Probably yes.
> But only after I get the ASS and SubRip decoder in libavcodec (currently
> working on it) and that I make use of them in av_sub.c.
> The end_time parsing might be implemented in av_sub.c (I haven't checked
> yet where is the best place).

Huh? If you use av_sub, the subtitle decoder certainly _must_ fill
in sub.end_display_time, so why would you need to parse them?

> Are you aware of anything else broken in lavf (mkv related) ?

There is a roundup issue on a webm test stream, but I haven't
even verified it is even valid.

> > > No, the demuxer generally don't have the subtitle display duration
> > > as a field. Matroska is really an exception here, but avi, nut and
> > > others don't have such a field.
> > 
> > That doesn't make it less silly to have to do a full sprintf->sscanf
> > roundtrip when the value we want is already there.
> 
> So are you saying that every single lavf user should implement this
> 
>   if (lavf->subtitle_duration)
>       duration = lavf->subtitle_duration;
>   else
>       sscanf("...", &duration);
> 
> instead of just this ?
> 
>   sscanf("...", &duration);
> 
> If so, I guess we have a different definition of what's silly...

No, I think that every application having to do the sscanf is silly.
Other formats might not already have the duration, but those could
e.g. use a parser to generate them.
However leaving the whole thing to a ASS decoder instead would be ok
as well.
However, implementing "ASS parser take 3" in MPlayer isn't reasonable.
If a FFmpeg ASS decoder doesn't work for us I guess reusing the subreader
code would be reasonable as well (but not really good though).