Adding loop functionality to Sampler (#1310) #1350
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1350 +/- ##
==========================================
+ Coverage 98.95% 98.97% +0.01%
==========================================
Files 203 203
Lines 22725 22829 +104
Branches 996 1005 +9
==========================================
+ Hits 22487 22593 +106
+ Misses 237 236 -1
+ Partials 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Tone/instrument/Sampler.ts
Outdated
| // get the current sources | ||
| this._activeSources.forEach((sourceList) => { | ||
| sourceList.forEach((source) => { | ||
| source.loopStart = loopEnd; |
There was a problem hiding this comment.
Looks like you accidentally set loopStart instead of loopEnd here.
Tone/instrument/Sampler.ts
Outdated
| @@ -197,6 +226,7 @@ export class Sampler extends Instrument<SamplerOptions> { | |||
| if (!Array.isArray(notes)) { | |||
| notes = [notes]; | |||
| } | |||
| const offset = defaultArg(0, this._loopStart); | |||
There was a problem hiding this comment.
I think you have this reversed, should be
const offset = defaultArg(this._loopStart, 0);
Tone/instrument/Sampler.ts
Outdated
| @@ -210,16 +240,22 @@ export class Sampler extends Instrument<SamplerOptions> { | |||
| const playbackRate = intervalToFrequencyRatio( | |||
| difference + remainder | |||
| ); | |||
| let duration = this._loop | |||
There was a problem hiding this comment.
nit: if duration isn't redefined, could be const instead of let
Tone/instrument/Sampler.ts
Outdated
| * @param loopStart The loop start time | ||
| * @param loopEnd The loop end time | ||
| * @example | ||
| * const sampler = new Tone.Sampler("https://tonejs.github.io/audio/berklee/guitar_chord4.mp3").toDestination(); |
There was a problem hiding this comment.
nit: this @example isn't a valid use of the API. i think it'd need to look like:
const sampler = new Tone.Sampler({
urls: {
A1: "https://tonejs.github.io/audio/berklee/guitar_chord4.mp3",
},
}).toDestination();
// loop between the given points
sampler.setLoopPoints(0.2, 0.3);
sampler.loop = true;
|
I've pushed fixes for the nits and linting errors. Let me know if anything else needs adjusting! |
This PR adds looping functionality to the Sampler consistent with Tone.Player. Looping is useful for Sampler instruments that need permanent continuous sound through crossfading such as the Violin.
An earlier PR by maximelebreton's from 2023 introduced basic looping but it lacked Unit testing as well as consistent behavior with Tone.Player such as getter's/setter's.