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

Kaldi-import: floor wav duration to milliseconds #971

Merged

Conversation

KarelVesely84
Copy link
Contributor

Hello Piotr,
what would you think about these suggestions ?

  • Kaldi-import: floor wav duration to milliseconds (I can reduce the comment, but it described how I got into it.)
  • bugfix in 'validate_for_asr()', the Supervision start may be delayed

Best regards
Karel

- bugfix in 'validate_for_asr()', the Supervision start may be delayed
@KarelVesely84 KarelVesely84 force-pushed the kaldi_wavdur_millisecond_validate branch from bd62b34 to 0d2c80a Compare February 8, 2023 15:04
pzelasko
pzelasko previously approved these changes Feb 8, 2023
Copy link
Collaborator

@pzelasko pzelasko left a comment

Choose a reason for hiding this comment

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

Thanks Karel, looks good. LGTM!

@pzelasko pzelasko added this to the v1.13 milestone Feb 8, 2023
@pzelasko
Copy link
Collaborator

pzelasko commented Feb 8, 2023

I see the checks just finished, can you fix the unit tests and formatting issues?

@KarelVesely84
Copy link
Contributor Author

Okay, the formatting is good now.

For the unit tests, I'll first need to learn how they work.
From the logs it seems that the tests are expecting the audio duration without the flooring to milliseconds.

I'll look into it tomorrow.
K.

@KarelVesely84
Copy link
Contributor Author

Hi Piotr,
the unit tests are now passing for me locally:
1564 passed, 41 skipped, 47 xfailed, 5 xpassed, 394 warnings in 199.65s (0:03:19)

It says I am a first-time contributor, so the workflows do not start automatically... :-)
K.

@pzelasko pzelasko merged commit 0e31189 into lhotse-speech:master Feb 9, 2023
@pzelasko
Copy link
Collaborator

pzelasko commented Feb 9, 2023

Thanks Karel!

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