mirror of
https://github.com/calesthio/OpenMontage.git
synced 2026-08-24 00:56:45 +08:00
fix(subtitle_gen): stop millisecond rounding from overflowing timestamps
_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.
This commit is contained in:
BIN
diagram.png
BIN
diagram.png
Binary file not shown.
|
Before Width: | Height: | Size: 5.8 KiB After Width: | Height: | Size: 5.2 KiB |
49
tests/tools/test_subtitle_timestamps.py
Normal file
49
tests/tools/test_subtitle_timestamps.py
Normal file
@@ -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)
|
||||
@@ -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}"
|
||||
|
||||
Reference in New Issue
Block a user