Skip to content

feat: allow simple filenames when synthesizing - #154

Merged
roedoejet merged 2 commits into
mainfrom
dev.ap/synthesis-improvements
Aug 25, 2026
Merged

roedoejet merged 2 commits into
mainfrom
dev.ap/synthesis-improvements

Conversation

@roedoejet

Copy link
Copy Markdown
Member

@semanticdiff-com

semanticdiff-com Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  fs2/cli/synthesize.py  12% smaller
  fs2/prediction_writing_callback.py  0% smaller
  fs2/tests/test_writing_callbacks.py  0% smaller

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.80%. Comparing base (f37c589) to head (8835153).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #154   +/-   ##
=======================================
  Coverage   48.80%   48.80%           
=======================================
  Files          27       27           
  Lines        2012     2012           
  Branches      231      231           
=======================================
  Hits          982      982           
  Misses        995      995           
  Partials       35       35           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roedoejet
roedoejet requested a review from joanise August 19, 2026 21:35
@roedoejet roedoejet changed the title feat: allow simple filenames when synnthesizing feat: allow simple filenames when synthesizing Aug 19, 2026
@joanise

joanise commented Aug 20, 2026

Copy link
Copy Markdown
Member

Tests failed in CI on push. Can you fix that?

Comment thread fs2/cli/synthesize.py Outdated
Comment on lines +602 to +604
" (e.g. 'LJ050-0269.wav') instead of the default, which also includes"
" the speaker, language, and other metadata"
" (e.g. 'LJ050-0269--LJ--eng--ckpt=100000--v_ckpt=100000--pred.wav')."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in StyleTTS2, I would remove the information that's redundant with what's shown in --output-dir. But yikes, I just reviewed the help for this command, the options are shown in an order that's not very logical. Out of scope for this PR, but we should do a thorough review of the order of all these synthesize parameters. text-to-wav has a better order, I would consult it in that review.

vocoder_model=vocoder_model,
vocoder_config=vocoder_config,
vocoder_global_step=vocoder_global_step,
simple_filenames=simple_filenames,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR put in my face just how much repetition we have in this file. Out of scope for this PR, but it would be nice to find a way to factor how some of this stuff that just gets repeated over and over again.

@roedoejet
roedoejet requested a review from joanise August 21, 2026 17:17

@joanise joanise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@roedoejet
roedoejet force-pushed the dev.ap/synthesis-improvements branch from 8835153 to d3a7809 Compare August 25, 2026 16:42
@roedoejet
roedoejet merged commit d3a7809 into main Aug 25, 2026
2 of 4 checks passed
@roedoejet
roedoejet deleted the dev.ap/synthesis-improvements branch August 25, 2026 16:42
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