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

MixedCut.truncate: fix the case when only PaddingCuts are left #1157

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

flyingleafe
Copy link
Contributor

I encountered another couple of bugs after simulating long conversations with ConversationalMeetingSimulator, cutting them into windows and loading the windows' audios:

  • If a window happened to be inside an offset region without any cuts, a PaddingCut was returned for which the audio (zeros array) could not be loaded, since num_samples property was not defined;
  • If it happened so that only PaddingCuts remained after truncation, this case was not properly handled (in the same way as "no cuts remained" case), which resulted in the IndexError exception at the last lines of truncate method (where we attempt to set SNR of first non-padding track to None)

This PR fixes those.

desh2608
desh2608 previously approved these changes Sep 18, 2023
@desh2608
Copy link
Collaborator

Thanks for catching this edge case. Can you also add a test which would have failed before making this change but passes now?

@flyingleafe
Copy link
Contributor Author

@desh2608 Sorry for such a delay, added the test.
The case with IndexError seems to be quite exotic - it replicates only when two or more PaddingCuts remained after truncation. Yet, apparently, cutting the output of ConversationalMeetingSimulator can lead to such cases.

@desh2608
Copy link
Collaborator

Thanks. LGTM!

@desh2608 desh2608 merged commit afc8eff into lhotse-speech:master Oct 11, 2023
10 checks passed
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.

None yet

2 participants