Enhance CutSet.mix() randomness and data utilization
#1315
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
osadj
reviewed
Apr 5, 2024
| to_mix = next(mix_in_cuts) | ||
| to_mix = self._maybe_truncate_cut(to_mix, target_mixed_duration, rng) |
There was a problem hiding this comment.
@pzelasko I just noticed this. We need to update the target duration for random sub-region selection in the while loop (like I had in the PR). Let's assume the target is 20s. The first noise cut is 10s long, and we enter the while loop. The second noise cut is 15s long so we can select a random sub-region of 10s from the 15s, however currently because we use the original target duration we select from 0. Do you agree?
There was a problem hiding this comment.
I think it makes sense. Can you make the change and verify it works as expected in another PR? Ideally add a unit test that would have been broken before the change is made, it will help maintaining this bit of code in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on #1313 and #1314: huge thanks to @osadj and @kamirdin for your contributions! While I was reviewing the PRs I noticed some subtle errors and figured it will be quicker to just combine your work and fix it in one go.