From cd85379423b4589826fa1b2ce973adca377ed6ea Mon Sep 17 00:00:00 2001 From: wm4 Date: Wed, 14 Aug 2013 13:23:37 +0200 Subject: [PATCH] sub: fix accidental subtitle overlaps The fix_overlaps_and_gaps() function in dec_sub.c fixes small gaps or overlaps between subtitle events. However, sometimes it could happen that the corrected subtitle events could overlap by 1ms due to bad rounding, making libass shift subtitles to reduce collisions. (The second subtitle will be shown above the previous one, even if both subtitles are visible only for 1ms.) sd_ass.c rounds the timestamps when converting to integers for unknown reasons. I think it would work fine without that rounding, but since I have no clue why it rounds, and since it could be needed to ensure correct timestamps with ASS subtitles demuxed from Matroska, I'd rather not touch it. So the solution is to use already rounded timestamps to calculate the new subtitle duration in fix_overlaps_and_gaps(). See github issue #182. --- sub/dec_sub.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sub/dec_sub.c b/sub/dec_sub.c index 8622d2c872..998cb0db7f 100644 --- a/sub/dec_sub.c +++ b/sub/dec_sub.c @@ -302,6 +302,8 @@ static void multiply_timings(struct packet_list *subs, double factor) } } +#define MS_TS(f_ts) ((int)((f_ts) * 1000 + 0.5)) + // Remove overlaps and fill gaps between adjacent subtitle packets. This is done // by adjusting the duration of the earlier packet. If the gaps or overlap are // larger than the threshold, or if the durations are close to the threshold, @@ -322,7 +324,10 @@ static void fix_overlaps_and_gaps(struct packet_list *subs) if (fabs(next->pts - end) <= threshold && cur->duration >= keep && next->duration >= keep) { - cur->duration = next->pts - cur->pts; + // Conceptually: cur->duration = next->pts - cur->pts; + // But make sure the rounding and conversion to integers in + // sd_ass.c can't produce overlaps. + cur->duration = (MS_TS(next->pts) - MS_TS(cur->pts)) / 1000.0; } } }