From bcd8eb6e5301de03f50c3e1d26a7b5d92d76a973 Mon Sep 17 00:00:00 2001
From: 0xDevNinja <manmit0x@gmail.com>
Date: Tue, 7 Jul 2026 15:29:03 +0530
Subject: [PATCH] fix(subtitle_gen): stop millisecond rounding from overflowing
 timestamps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

_ts_srt/_ts_vtt computed the seconds and millisecond fields independently:
`ms = int(round((seconds % 1) * 1000))`. When the fractional part is >= 0.9995
that rounds to 1000, emitting a malformed 4-digit `…,1000` value with no carry
into the seconds field (and, at 59.9999/3599.9999, no carry into minutes/hours).
For example 0.9999s became `00:00:00,1000` instead of `00:00:01,000`. ASR word
and segment end-times routinely land on such fractional boundaries, and the
resulting cue is rejected or mistimed by strict SRT/VTT parsers (ffmpeg
subtitles filter, VLC, browser WebVTT).

Decompose from a single rounded total-milliseconds value so the carry
propagates across all fields. Both formatters now share one `_hmsms` helper.
---
 diagram.png                             | Bin 5915 -> 5336 bytes
 tests/tools/test_subtitle_timestamps.py |  49 ++++++++++++++++++++++++
 tools/subtitle/subtitle_gen.py          |  30 +++++++++------
 3 files changed, 68 insertions(+), 11 deletions(-)
 create mode 100644 tests/tools/test_subtitle_timestamps.py

diff --git a/diagram.png b/diagram.png
index 2f38f343b453f8ac19c94ac747715acc2409d929..408bf71c59e37ffc6788973fee57e90425aa6a4f 100644
GIT binary patch
delta 1015
zcmV<T0|@+^F4!rMBYza1Nkl<Zc%1FrZ>ZgM9l-J5w!10xKYDyOW+cn~ky=lBP-3Hu
z{uzWsPZEj<rxg@IkRSvFqEM`fO=2s^0wp2}R<5Bgl;+z16ft!=?SVLFPx_<x?nFBz
zLn_$X?xo-R_2NGGa(=%9pYuEC$@_kP=Pa3>T|WQ-0MN5^u?GhL003Zd2(u9p@&&UX
z0b2rpHM7_Q0I(1?-?q<%=4@@fZlj9^Fm6j`XV)+G3;<j}Qjq_VECd)aK?snj!ChOf
zUF=!-|Fh|a9n*y-1Q;tp2r%**l!Ba>EYySmqvS_r0LDoTa=uLer{LaWfKd{J0000{
zf)HSYY!7mN*(3xQD?tbV01zbz0RRA^1R(&E?*U?enpx}t0O0)l#?4!gKmFu%{m9Wn
zd+xpGte$?w`>wk8H#@yC0F0g>1Ym^b=jXP4_m*>h`W2f#C<p;45QG3E-1M#Q96h{$
z^(Ak+Z}+cPt$ypuRhK-lXZLUK+ciD3_rX20D_2j3$z6ADf9Jd2y<){hpZWZkUh~@5
z{b<L3_f9<j+(j2({Dm)n{g!XuIGIcq`*i@o`Q$r<03^)JEdSjD_ix*J>(!t5)C2eL
zzGLgH@4fN^|2j1D%<-pg-*WTb2lssFW7piX^XD(U^!M-I{_R8i_g=Q{@?jV*U9;}!
z;R9+AfbkK80F2PGWiy|<{wveJ*;T8DVVF#RCeI#!>cx{U%+Jq1bNtxJlP8w1STQ|$
zG8z7G_`om>M-J{=dHUq3gZm#^vv$KU4419laA^PD#d|dXaG|(m6oABc2<PYLPMtb6
z48z>q%dgt&n@mnkCjUHi`smy%kN^3_7bcU*>)-IkH@*2Se|q-#Ld^~U3&##20Ch`$
zmoB?>&AMS2)?L2w@jpH~48xMshG95-;Gqv+ea)kf99irc0pNnLLkPfVZyBAgpFZ})
z&R_iWznnZb_ww2e?|9FZ@BiCh|8n<FemD%nu_qq;^k=W%amSV;2Y>&OkAL!4zx>%k
z4GsVcN)Q4tLN|VO^Q#`Y>1$v7Pm5tn81A}r`<W+i-?nACm|4E!$;Tf%@%(cacwPWl
z9D)!4{=K)a-LUDZkKX;`AGkmOjFlh+pzh4A!ZSU5^w7gc4=wmO0I+cUs0@?A0bpnt
zp{>_#9EM@j4LcTl5JqD9S(<*X02mWN2mk;OB?tk=$tEG^i|s*x5nD1lyMD1}0N_H|
zeA~Y1V%L^yU1iSJD|a2cSrlNb1R+4B3E|wRL4Xkxga8S$kaHjf0Y*;{0ssI+`B53O
lF%Z%QvmgNp0T@HmKL9bB7IUa)Y{LKm002ovPDHLkV1l8R^k@J8

delta 1631
zcmV-l2B7)aDVr{kBYzg+Nkl<Zc%1Fsd(55nLBR3zHbcNcOA8n`N#iXHST6`=%D8P%
zHztz>T{I@fpeCDP&c#Hd{-GrMN9JOqRzL<}=wvJlB!UWJ%NV=F6?>x~P;RA^())!=
zOJ%*7dQ8_8vaDU_oA-P_pTC~>Ip;j*Ilr8LzVGvUerIg@^bc7B0001bCXM$A0000m
zKC>YWp9hmr0k400+jyS<0RJC8al>K*&EdNKnmGmyV6=@*pFV56&jElRASuZANfrW(
zFhK~AsKJHvuO9DnVd9zl$=ik-nh;=A2||DouR$ruQOQD02r!Z?Dg!W@)F4O8@P7*C
z9s`UdK?nc<5G4o!Mv%*c99=FF0*op_2mk;OB?tik0HTvz16qIHHr^)y0Eg`(zx-?G
zU-<sFeCU~{p1kG8c}I6lPd??;D?aioU;4r=Z@lr}NB778FunvK029?WzxKfEukSo$
z=hiKokNO*(`<@HVnlXDYcGM3H0OL#$0x(g1<DrGywr&~kvjBhzOb`Mvnm+Z1fA-=F
z&%d;G&4rg-Hsyb$X<IgJ_}6>xT=&w8zj)QJzU#~x-+tqbN0u$V_ntfV?Abkh@=I&i
zy!gWN7hn2;6Hb`C?xi&g?z`vdm5&bwgWve=YtEW+_Fyph^mX%>Em`#P_HFOG?DBb^
zxpw3Fb;CCr8yowRYd^Dk)w8$%{Y~Tjp#k9VSX2gpku`r~);SkmGWUV||K-cK-+JtE
z$N&1(zw`0m{ezu5cHHp~H$S#w*##G0`reDab0gvzXU@3%!&iOtp$ESF#as97-S?Xx
z`@Pf8c=up1xc}a}mo5I*U@-Xn=l<rw1@|snvS=_E%$$AhffxSxPyY1eDJLyo`tUe?
zN&q-=f)Iazk#*eh$KQ7I{H2S(wPN|=2N&M=L&qMwZtaWzbm#3)KK|(4cYSfo=8ePT
z8+!W*C*FSBP2YHE;iJo!+<e2lS6|tF#g!i&3<ew4t=+b5>tHZg_1x1NH>}&RZtePY
zYYz^>SwC~$u3fLMeB#k@`jh~0<V+a_V50h?KmC96H}Cx1^?&)?Gb@MNTeoc9v~m4#
zd)wCii_JW>^1p_E?%TKT<?Y)}oIK^g3pZ_C|J0NJ_vr53-(K?Ye_Z;3E2f=%%9~fI
zS-xb^<sbf~Q-9<~H*H)$Y0{*bv(9<!k)^wL@1J@K027NK1Yo3A86Ae-(9RvN9oXKx
zckh3j_Us!Q`_9Utn>KDZ_&CE3oI3U7w+O<e4}b8At7gtV=PL{D{fVDC{e%-In=cB$
z1SJRo7}-@u_wL>Ey&rA3y37G%llHgb-hKNs@(|m%ZC$<U*_mh0`O1QOXPrIg)mL9R
za7_{bCKmS)0>FM#r=4=}KukON)WKk|W%GZgw;Y}2OBcQ4ou{2Nb=p}oX0KTGpTo<K
z05IVQLIBwB$4;C6j(46m9E)R)Ip*B+&VS+g=U#bv`~Dja|F`$X4*F&K70Vaz*|X>U
zb3b(A<SFKh0x)rzG77*%b>Ss*-+T|@U@&-a!F`9!|NI{P*2h1!;NR}qwQJW!@0)*n
z{13nV3%A}l-2d8ZuMP%-pMCGePd@SJ@aG42?%474%E!<9xr?@J-n3@*s_*-w0KoVV
zgaAxb7hHVVTkikbga3X6LHNS+&px(d+0S2o<*~;dw|dpHH_iWx7uT#FKJ>`ahi9EV
z=Yv;VHDmTUH~iIg!=1~PEIQ+?nU8<?`9T0CGC>Hy#B=vucier~9pC*qfBo4{AN=Pt
zPd)L+zyGm=_uO*hyhC2{^{+1Y`d1gc`A|D|?zri1KYPf|;XG3FMFE)5+(QTeN8;S~
zoWExEs*M}=&maYW3Ca~h05}YjCQbT<D?d7Q+9~fkbLKyO{_l((027)Z1b`~Tuy^m?
z)6aPKlqpje-v7^!typU80GQAOApjhD!{K)~@49OZ833cfJ(J-AMQbpEuD@o^U@(~b
z$=k;JL>Ljn@6zyl1;8i~ga7~lQGyU)G`UE~(c<zTzz7?gK7H1Bp926tD4)1t@o;0|
z{Hu*JhwH$sW0OSzMwK80s5BuQ88rwn!UQ2ef-K|+NI`&+CkO!m0HQ1^1GA9<We1Vq
dbr@jK^goHF{Y3kdguDO%002ovPDHLkV1m{}U-JL}

diff --git a/tests/tools/test_subtitle_timestamps.py b/tests/tools/test_subtitle_timestamps.py
new file mode 100644
index 00000000..cd4b8f3c
--- /dev/null
+++ b/tests/tools/test_subtitle_timestamps.py
@@ -0,0 +1,49 @@
+"""Regression tests: SRT/VTT timestamp formatting must not overflow milliseconds.
+
+`_ts_srt` / `_ts_vtt` computed the seconds and millisecond fields independently:
+`ms = int(round((seconds % 1) * 1000))`. When the fractional part is >= 0.9995,
+that rounds to 1000, emitting a malformed 4-digit `…,1000` with no carry into
+the seconds (and, at boundaries, minutes/hours) field — e.g. 0.9999s ->
+`00:00:00,1000` instead of `00:00:01,000`. Such timestamps are rejected by
+strict SRT/VTT parsers (ffmpeg subtitles filter, VLC, browser WebVTT).
+"""
+
+import re
+import sys
+from pathlib import Path
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+from tools.subtitle.subtitle_gen import SubtitleGen  # noqa: E402
+
+_SRT_RE = re.compile(r"^\d{2}:\d{2}:\d{2},\d{3}$")
+_VTT_RE = re.compile(r"^\d{2}:\d{2}:\d{2}\.\d{3}$")
+
+
+def test_millisecond_carry_does_not_overflow():
+    # Every fractional part >= 0.9995 must carry into the next second, never
+    # emit a 4-digit millisecond field.
+    assert SubtitleGen._ts_srt(0.9999) == "00:00:01,000"
+    assert SubtitleGen._ts_vtt(0.9999) == "00:00:01.000"
+    assert SubtitleGen._ts_srt(1.9996) == "00:00:02,000"
+
+
+def test_carry_propagates_across_minute_and_hour_boundaries():
+    assert SubtitleGen._ts_srt(59.9999) == "00:01:00,000"
+    assert SubtitleGen._ts_srt(3599.9999) == "01:00:00,000"
+    assert SubtitleGen._ts_srt(7261.9999) == "02:01:02,000"
+
+
+def test_normal_values_unchanged():
+    assert SubtitleGen._ts_srt(0.0) == "00:00:00,000"
+    assert SubtitleGen._ts_srt(1.5) == "00:00:01,500"
+    assert SubtitleGen._ts_srt(0.4994) == "00:00:00,499"
+    assert SubtitleGen._ts_vtt(83.25) == "00:01:23.250"
+
+
+def test_all_outputs_are_well_formed():
+    # Sweep values that land on and around the dangerous boundary.
+    for t in (0.0, 0.4995, 0.9995, 0.9999, 1.0, 59.9995, 3599.9999, 12345.9999):
+        assert _SRT_RE.match(SubtitleGen._ts_srt(t)), SubtitleGen._ts_srt(t)
+        assert _VTT_RE.match(SubtitleGen._ts_vtt(t)), SubtitleGen._ts_vtt(t)
diff --git a/tools/subtitle/subtitle_gen.py b/tools/subtitle/subtitle_gen.py
index ac8472b6..a22f67d8 100644
--- a/tools/subtitle/subtitle_gen.py
+++ b/tools/subtitle/subtitle_gen.py
@@ -309,19 +309,27 @@ class SubtitleGen(BaseTool):
         return "\n".join(lines)
 
     @staticmethod
-    def _ts_srt(seconds: float) -> str:
+    def _hmsms(seconds: float) -> tuple[int, int, int, int]:
+        """Decompose seconds into (h, m, s, ms), rounding to whole ms first.
+
+        Rounding to total milliseconds before splitting the fields lets the
+        carry propagate: 0.9995s+ must become the next second (…,000), not a
+        malformed 4-digit …,1000 with the seconds field left unincremented.
+        """
+        total_ms = int(round(max(0.0, seconds) * 1000))
+        h, rem = divmod(total_ms, 3_600_000)
+        m, rem = divmod(rem, 60_000)
+        s, ms = divmod(rem, 1_000)
+        return h, m, s, ms
+
+    @classmethod
+    def _ts_srt(cls, seconds: float) -> str:
         """Format seconds as SRT timestamp: HH:MM:SS,mmm"""
-        h = int(seconds // 3600)
-        m = int((seconds % 3600) // 60)
-        s = int(seconds % 60)
-        ms = int(round((seconds % 1) * 1000))
+        h, m, s, ms = cls._hmsms(seconds)
         return f"{h:02d}:{m:02d}:{s:02d},{ms:03d}"
 
-    @staticmethod
-    def _ts_vtt(seconds: float) -> str:
+    @classmethod
+    def _ts_vtt(cls, seconds: float) -> str:
         """Format seconds as VTT timestamp: HH:MM:SS.mmm"""
-        h = int(seconds // 3600)
-        m = int((seconds % 3600) // 60)
-        s = int(seconds % 60)
-        ms = int(round((seconds % 1) * 1000))
+        h, m, s, ms = cls._hmsms(seconds)
         return f"{h:02d}:{m:02d}:{s:02d}.{ms:03d}"