[MPlayer-dev-eng] [PATCH] Read normal duration when demuxing subtitles through lavf (v2)

[Alexander Strasser]

Reimar Döffinger wrote:
> On Fri, Jul 27, 2012 at 07:21:45PM -0700, Philip Langdale wrote:
> > Hi all,
> > 
> > Currently, mplayer only looks at convergence_duration when
> > using lavf to demux subtitles. Some containers use the
> > normal duration field for subtitles (eg: mov_text), so we
> > should read that if the convergence_duration is not used.
> > 
> > Thanks,
> > 
> > --phil
> 
> > Index: libmpdemux/demux_lavf.c
> > ===================================================================
> > --- libmpdemux/demux_lavf.c	(revision 35050)
> > +++ libmpdemux/demux_lavf.c	(working copy)
> > @@ -653,9 +653,14 @@
> >          priv->last_pts= dp->pts * AV_TIME_BASE;
> >          // always set endpts for subtitles, even if AV_PKT_FLAG_KEY is not set,
> >          // otherwise they will stay on screen to long if e.g. ASS is demuxed from mkv
> > -        if((ds == demux->sub || (pkt.flags & AV_PKT_FLAG_KEY)) &&
> > -           pkt.convergence_duration > 0)
> > -            dp->endpts = dp->pts + pkt.convergence_duration * av_q2d(priv->avfc->streams[id]->time_base);
> > +        if (ds == demux->sub || (pkt.flags & AV_PKT_FLAG_KEY)) {
> > +            int64_t duration = 0;
> > +            if (pkt.convergence_duration > 0)
> > +                duration = pkt.convergence_duration;
> > +            else if (pkt.duration > 0)
> > +                duration = pkt.duration;
> > +            dp->endpts = dp->pts + duration * av_q2d(priv->avfc->streams[id]->time_base);
> 
> The intention was that endpts will only be set if the duration is > 0.
> Subtitles never disappearing is better than them never appearing
> because pts == endpts.

  I was just thinking about that. But am I the only one who finds the
comment misleading?

  Anyway, I would propose to just pull the dp->endpts assignment into
both branches of the inner if and directly use the corresponding duration
member.

  Any objections? I would edit that myself when testing, so no need for
a resending the patch.

  Alexander