Adds split_once to Source #675
Conversation
split_once can be used chop a source into two sources without duplicating the first. Those sources can then not be played simultaneously. This is usefull for adding a fadeout at the end of a song for example. Though we could later specialize fadeout at end. The implementation uses a `SplitAt` struct, `split_once` returns an array of two of those. In the future we can introduce methods like `split` that return a vec of SplitAt's allowing users to chop a source into multiple sections
There was a problem hiding this comment.
Looks good. I do not like that first_span_ stuff but this can be cleaned up later.
Alternatively, if the motivation is to support for fade-out, we can modify fade-out to start at some later moment (it would pass-through until it's time to ramp down). Maybe we can even have arbitrary "automation" lane, with piece-wise gain function (can be even smoothed with splines :)
src/source/mod.rs
Outdated
| @@ -498,6 +500,15 @@ where | |||
| skippable::skippable(self) | |||
| } | |||
|
|
|||
| /// returns two sources, the second source is inactive and will return | |||
| /// `None` until the first has passed the split_point. | |||
| fn split_once(self, at: Duration) -> [SplitAt<Self>; 2] | |||
There was a problem hiding this comment.
Why not split_at? To me it sounds easier to understand.
There was a problem hiding this comment.
named after: https://doc.rust-lang.org/std/primitive.str.html#method.split_once, we might later want a split method that takes a vec of durations and returns a vec of SplitAts
We can do split_once_at?
There was a problem hiding this comment.
split_once in std uses content of the sequence as delimiter. So, number of pieces in the result is unpredictable, the delimiter can occur more than once. Therefore, in that case it is necessary to distinguish the method that uses only first delimiter from one that returns all the fragments. In this case of the Source splitter, the delimiter is time which is unique.
There was a problem hiding this comment.
Good point however I think there is still a good case here: we are returning the results on the stack (via the array) allows unpacking & increasing performance, for example:
let [start, end] = some_source.split_once(..);is easier and safer (hidden panic in [0] & [1] that can not be checked compile time) then:
let split = some_source.split(..);
let start = split[0];
let end = split[1];
src/source/mod.rs
Outdated
| @@ -622,6 +635,9 @@ impl fmt::Display for SeekError { | |||
| #[cfg(feature = "wav")] | |||
| SeekError::HoundDecoder(err) => write!(f, "Error seeking in wav source: {}", err), | |||
| SeekError::Other(_) => write!(f, "An error occurred"), | |||
| SeekError::SplitNotActive => { | |||
There was a problem hiding this comment.
FragmentNotActive (or FragmentIsNotActive), maybe...
There was a problem hiding this comment.
By using the word split I hope to have users associate the error with the split_once function and SplitAt struct. Fragment is another term so that association is gone.
What is the problem with SplitNotActive? I agree its not a very clear term.
There was a problem hiding this comment.
Yes, maybe. I thought that "fragment" maybe become useful in other places where partial sources will be used...
There was a problem hiding this comment.
English is not my native language but to me it feels like "split" normally refers to the location or cause of divide, not the parts it separates.
There was a problem hiding this comment.
I agree with your motivation, Ill rename it to Segment (fragment means a small bit of a larger whole and these segments can be quite large)
That was the one of the motivations for creating this. It seemed valuable to have a method of splitting a source into two "mutable" sections for other effects. I thought about a fadeout_end source but it would only be marginally more efficient. It is easier to use/find but I think Sink/ the new Player should help out with that. The Player should get a fadeout_end setting. |
This sounds interesting though I do not fully get how it should work yet. |
|
ill fixup your suggestions later, gotta get to bed now 🌔 edit: oh and thanks for a clear and useful review! |
|
ready for review again |
split_once can be used chop a source into two sources without duplicating the first. Those sources can then not be played simultaneously. This is usefull for adding a fadeout at the end of a song for example. Though we could later specialize fadeout at end.
The implementation uses a
SplitAtstruct,split_oncereturns an array of two of those. In the future we can introduce methods likesplitthat return a vec of SplitAt's allowing users to chop a source into multiple sections