Skip to content

changes main.py & test_srt.py issue #11 - #12

Open
hirendra84 wants to merge 2 commits into
langswap-app:mainfrom
hirendra84:main
Open

changes main.py & test_srt.py issue #11#12
hirendra84 wants to merge 2 commits into
langswap-app:mainfrom
hirendra84:main

Conversation

@hirendra84

Copy link
Copy Markdown

feat(subtitles): migrate subtitle generation from SRT to WebVTT

  • Rename generate_srt_files() to generate_vtt_files()
  • Change output extension from .srt to .vtt
  • Add WEBVTT header and WebVTT-compliant formatting
  • Use dot-separated timestamps and speaker cue tags
  • Remove segment numbering
  • Update api, main, gradio demo, and tests
  • Add VTT-specific test assertions

@shigabeev

Copy link
Copy Markdown
Contributor

Hey @hirendra84 thank you for you PR. Sorry for a late reply. I'll check it and come back here to see if I can merge it.

@shigabeev

Copy link
Copy Markdown
Contributor

Thanks for picking this up, @hirendra84. Direction is right, but this can't merge as-is — I think you may have missed a commit.

The diff only touches main.py and tests/test_srt.py. generate_vtt_files() doesn't exist yet: langswap/translation_pipeline.py:187 still defines generate_srt_files() and still writes SRT (numbered cues, comma timestamps, .srt filenames). So main.py raises AttributeError at the last stage and the new test fails. langswap/api.py:132, langswap/api.py:193 and gradio_demo.py:141 also still call the old name.

The point of #4 is the speaker tags, not the rename — that's the whole reason VTT is worth moving to. The data is already there: TranslatedTextedSegment.speaker (langswap/pipeline_models/models.py:34) is populated by the ASR clients, it's just never written out. So the change belongs in generate_srt_files():

  • WEBVTT\n\n header
  • drop the cue numbers
  • , → . in timestamps
  • wrap text as <v {segment.speaker}>
  • .srt → .vtt in the two get_file() calls
  • Could you push that plus the api.py / gradio_demo.py call sites? Worth renaming tests/test_srt.py → test_vtt.py too.

One note on the test: asserting SPEAKER_00 passes even if the speaker path is broken, since every ASR client defaults to that literal. Build the fixture with two distinct speakers and assert both appear — otherwise the test doesn't test the feature.

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.

2 participants