Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to Timing Overlap Issue #608 and #621 #816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

FredBill1
Copy link

Currently the function iterate_subtitles does not compute the start and end time correctly for each combined subtitle segment. All it does is selecting the start and end time of the first segement in the combined segement, without considering the following segments or segment breaks. The start and end time of each word is ignored, causing the --max_line_count and --max_line_width to return segements with overlapped timing, as stated in issue #608 and #621.

times.append((segment["start"], segment["end"], segment.get("speaker")))

whisperX/whisperx/utils.py

Lines 281 to 284 in f2da2f8

for subtitle, _ in iterate_subtitles():
sstart, ssend, speaker = _[0]
subtitle_start = self.format_timestamp(sstart)
subtitle_end = self.format_timestamp(ssend)

yield subtitle_start, subtitle_end, prefix + subtitle_text


I added a piece of code to compute the start and end time of the combined segment based on the time of each word in the segment. But I'm not sure how to utilize the times yielded by iterate_subtitles.

@issaMbarki
Copy link

I edited the iterate_subtitles function and it gave me really good result now with max_line_count and max_line_width, once I'll have sometime I'll create a pull request

@tormki-uio
Copy link

This pr worked well for me, as we've been struggling with issues around max_line_width and max_line_count created duplicated timestamps for split segments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants