[MPlayer-dev-eng] Re: [PATCH] SSA/ASS subtitles support

[Uoti Urpala]

On Sun, 2006-06-25 at 17:09 +0400, Evgeniy Stepanov wrote:
> 3. cosmetics.patch

IMO handling a separate patch for this is much more trouble than
including it in the main patch (yes I know some people asked to have it
separately...).

> An unrelated issue: mencoder-ass.patch was included in mplayer-ass.patch by 
> mistake. Fixed versions attached.

I haven't looked at the latest versions yet, but remembered an issue
that I forgot to mention in my previous comments:

+               images = ass_render_frame(vf->priv->ass_priv, ass_track,
(pts+sub_delay) * 1000);

It would be better to round the timestamp value instead of the current
truncating cast to long long. If the original video was mkv with 1 ms
timestamp precision and the subtitles were timed to the exact frame
timestamps it could matter. Converting the original timestamp to double
and then back with the truncating conversion here can produce a
smaller-than-original value. It'll happen for almost all timestamps if
the fpu is in round-down mode and for some in round-to-nearest mode
(most likely case). Adding rounding here is guaranteed to reproduce the
original value as long as it isn't so insanely huge it would exceed the
precision of doubles.